Skip to content

[Add] Consequences of identity for monoids #2692

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

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

jmougeot
Copy link
Contributor

Introduce reasoning combinators for monoids.

This PR is intended to focus on the Monoid.Reasoning filr as the discussion regarding the Semigroup.reasonig file is in the PR : #2688

I went with the names from agda-categories since they seemed to make sense.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Apr 10, 2025

Probably the 'best' thing to do is to mark this as blocked (on #2688 ), otherwise you'll end up with merge conflicts when that PR gets merged, and indeed to start afresh with this PR being on a branch based on jmougeot/semigroup (or else: rebase this one against that branch). That keeps both the locality of each PR smaller, but also the (sequential) dependency between them.

@jamesmckinna jamesmckinna added addition status: blocked-by-issue Progress on this issue or PR is blocked by another issue. labels Apr 10, 2025
@jamesmckinna jamesmckinna added this to the v2.3 milestone Apr 10, 2025
@jamesmckinna
Copy link
Contributor

Suggest corresponding title change for the PR in the spirit of that for #2688 ...

@jmougeot jmougeot changed the title [Add] reasoning combinators monoids [Add] Consequences of identity for monoids Apr 10, 2025
@jmougeot jmougeot changed the title [Add] Consequences of identity for monoids [Add] Consequences of identity for monoids Apr 10, 2025
@MatthewDaggitt
Copy link
Contributor

Given that this is not properly rebased, and the naming scheme is different from the semigroup PR and that I need to make a release candidate tomorrow, I'm going to remove this from the v2.3 milestone tomorrow @JacquesCarette. If you would like this to urgently get in then please bring it up to date.

@JacquesCarette
Copy link
Contributor

I will try.

@jmougeot jmougeot force-pushed the monoid-reasoning branch from 1ae1e27 to dab0763 Compare July 3, 2025 20:35
Copy link
Contributor

@JacquesCarette JacquesCarette left a comment

Choose a reason for hiding this comment

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

I think this is in excellent shape now.

@JacquesCarette
Copy link
Contributor

@jmougeot pushed through the work (thanks!)

Yes, the naming scheme here is quite different. However, I claim that the names in this PR make sense. The ones for original semigroup PR didn't (i.e. the ones based on push and pull which are quite arbitrary). When you read a proof that uses the names here, you get a 'semantic sense' of what the proof does. They've also been used a lot in agda-categories and adopted in other libraries as well. And we've agreed that the current names in semigroup are an unsatisfying compromise, i..e the names are at best 'not wrong'.

I'm fine if this doesn't make it in to v2.3 -- but it shouldn't be because of the names!

@jamesmckinna jamesmckinna removed the status: blocked-by-issue Progress on this issue or PR is blocked by another issue. label Jul 4, 2025
@jamesmckinna
Copy link
Contributor

I'm (qualified) happy with this, but would like us to revisit the names before v3.0, when we can break/deprecate to improve matters. My concern would be that we choose bad names, use them, extensively even, and then have more knock-on viscosity once we find the 'right' ones... but that is, here, I think outweighed by having them available for experiment, so their utility (and memorability!) can be experimented with...
... but as I'm on cell signal, I'll defer the heavy lifting and final approval to the release manager... ;-)

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

Successfully merging this pull request may close these issues.

4 participants