Skip to content

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