Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add toMap and fromMap to Data.Set. - Retry #31

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

milesfrain
Copy link
Contributor

A rebased attempt at #21
Hopefully this works now with updated CI

@milesfrain
Copy link
Contributor Author

Looks like this is good to merge now.

I'd also be happy to see this convenience function added too, but that can be a separate PR.

annotateSet :: forall a b. (a -> b) -> Set a -> Map a b
annotateSet f = mapWithIndex (\k _ -> f k) <<< toMap

@np, feel free to rebase your original PR, or we can go with this one if that's more convenient.

@hdgarrood
Copy link
Contributor

Looks good, but is the typelevel-prelude dependency being used at all? If not it should be removed from bower.json.

@milesfrain
Copy link
Contributor Author

According to CI, it's unnecessary.

@np
Copy link

np commented Sep 7, 2020

I'm good with this PR.

👍 for annotateSet.

@milesfrain milesfrain mentioned this pull request Sep 7, 2020
@JordanMartinez
Copy link
Contributor

Assuming CI passes, we can merge this in.

@JordanMartinez
Copy link
Contributor

@milesfrain Sorry, but could you rebase this on top of current master? Seems CI is failing because Travis CI is still being used rather than GH Actions.

@milesfrain
Copy link
Contributor Author

milesfrain commented Dec 20, 2020

Compressed history with a force push to make future rebases easier.

Also noticed that pulp test fails locally on both this commit and master. So I'm not sure how master previously passed CI.

Attempting to check CI on master with #40

@JordanMartinez
Copy link
Contributor

CI now fails with MonadZero deprecation notices (easy to fix) and this error:

1/1 TypesDoNotUnify] test/Test/Data/Map.purs:47:15

  47    arbitrary = elements $ A :| [B, C, D, E, F, G, H, I, J]
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  
  Could not match type
  
    NonEmpty Array
  
  with type
  
    NonEmptyArray
  
  while trying to match type NonEmpty t1 SmallKey
    with type NonEmptyArray t0
  while checking that expression (NonEmpty A) [ B
                                              , C
                                              , D
                                              , E
                                              , F
                                              , G
                                              , H
                                              , I
                                              , J
                                              ]
    has type NonEmptyArray t0
  in value declaration arbSmallKey
  
  where t0 is an unknown type
        t1 is an unknown type

@hdgarrood
Copy link
Contributor

That error is because of a breaking change in an upstream dependency (namely quickcheck). I expect it was passing the last time we merged something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants