Skip to content

Conversation

@QueenJcloud
Copy link

This pull request adds a doctest for the Op type in cats.core.
The doctest provides a simple usage example demonstrating how Op can be used for contravariant composition.

@armanbilge
Copy link
Member

Thanks for the PR! The CI failed due to a formatting issue, try running scalafmtAll in sbt.

@satorg
Copy link
Contributor

satorg commented Oct 20, 2025

I think it would be nice if it would be possible to explain in the docs why someone may want/need this Op class in the first place. It seems there's no documentation for Op available in Cats whatsoever (or it is not that easy to find).

The class was introduced in #2217 (Add dual categories). As far as I concerned, the "Dual Category" term comes from "Category Theory" and in a nutshell refers to an ability to flip the direction of an arrow.

But it doesn't explain why someone may need this class specifically, because its compose depends on Compose which in turn already has a reversed operation andThen (and also provides handy >>> and <<< operators whereas Op doesn't).

@QueenJcloud
Copy link
Author

I think it would be nice if it would be possible to explain in the docs why someone may want/need this Op class in the first place. It seems there's no documentation for Op available in Cats whatsoever (or it is not that easy to find).

The class was introduced in #2217 (Add dual categories). As far as I concerned, the "Dual Category" term comes from "Category Theory" and in a nutshell refers to an ability to flip the direction of an arrow.

But it doesn't explain why someone may need this class specifically, because its compose depends on Compose which in turn already has a reversed operation andThen (and also provides handy >>> and <<< operators whereas Op doesn't).

I’ve addressed both feedback, ran scalafmtAll to fix the formatting issues, and added documentation explaining the purpose and usage of the Op class.
Kindly take a look when you can and let me know if there’s anything else I should adjust or the next steps. Thank you

@QueenJcloud
Copy link
Author

Thanks for the PR! The CI failed due to a formatting issue, try running scalafmtAll in sbt.

I’ve addressed both feedback, ran scalafmtAll to fix the formatting issues, and added documentation explaining the purpose and usage of the Op class.
Kindly take a look when you can and let me know if there’s anything else I should adjust or the next steps. Thank you

restored correct package structure (package cats; package data), simplified class-level Scaladoc for better clarity, replaced previous example with a more illustrative dual composition example
@QueenJcloud
Copy link
Author

Hi @satorg,

I just saw that the CI flagged a missing header for Op.scala, but all tests passed successfully. I’ll wait for your feedback before making any further changes, just to be sure I handle it correctly. Please let me know the next step when you can. Thanks!

@QueenJcloud
Copy link
Author

hello @satorg,

I have made corrections as per the headercheck. Please kindly check and let me know the next step when you can. Thanks!

@QueenJcloud
Copy link
Author

QueenJcloud commented Oct 23, 2025

Hi @satorg, can you please recheck and let me know...Thanks.

Copy link
Contributor

@satorg satorg left a comment

Choose a reason for hiding this comment

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

Thank you!

@QueenJcloud
Copy link
Author

Thank you!

You’re welcome!
Glad to make the update, thank you for reviewing!

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.

3 participants