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 unorderedFoldMapA method #4662

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Add unorderedFoldMapA method #4662

merged 5 commits into from
Oct 10, 2024

Conversation

m50d
Copy link
Contributor

@m50d m50d commented Oct 8, 2024

Slightly trivial, but it's a method I'd expect to be there, a natural counterpart of foldMapA.

johnynek
johnynek previously approved these changes Oct 8, 2024
Copy link
Contributor

@johnynek johnynek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems worth having to me. That said, I think CommutativeApplicative may be underused, so we there could be instances we don't label.

@satorg
Copy link
Contributor

satorg commented Oct 8, 2024

@m50d , thank you for the PR!

Would you be open to add a rule to UnorderedFoldableLaws? Since unorderedFoldMapA can be implemented in terms of unorderedFoldMap, then the rule could look like "unorderedFoldMapA is consistent with unorderedFoldMap", or something like that.

@m50d
Copy link
Contributor Author

m50d commented Oct 9, 2024

@satorg I've added an identity law, it feels a bit overkill to add a full consistency law given that we don't have one for foldMapA (or maybe I should be adding them for both?)

@satorg
Copy link
Contributor

satorg commented Oct 9, 2024

Thank you @m50d!
You may also want to add this law to the corresponding rule set in laws/src/main/scala/cats/laws/discipline/UnorderedFoldableTests.scala.
Without that the law won't get checked.

@m50d
Copy link
Contributor Author

m50d commented Oct 9, 2024

Ah indeed - thank you!

@satorg
Copy link
Contributor

satorg commented Oct 10, 2024

Thank you, @m50d !

@satorg satorg merged commit 478e1a4 into typelevel:main Oct 10, 2024
16 checks passed
@m50d m50d deleted the unorderedfoldmapm branch October 15, 2024 00:26
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.

4 participants