Skip to content

Inconsistency: map-compose vs. map-∘ #509

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

Closed
gallais opened this issue Oct 20, 2018 · 12 comments
Closed

Inconsistency: map-compose vs. map-∘ #509

gallais opened this issue Oct 20, 2018 · 12 comments

Comments

@gallais
Copy link
Member

gallais commented Oct 20, 2018

In Data.List.Properties we use map-compose whereas in Data.Vec.Properties we use map-∘.

@gallais
Copy link
Member Author

gallais commented Jan 2, 2020

And in Codata.* we use map-map-fusion.

@MatthewDaggitt MatthewDaggitt added this to the v2.0 milestone Jul 11, 2021
@jamesmckinna
Copy link
Contributor

Suggest map-∘ throughout?

@gallais
Copy link
Member Author

gallais commented Sep 14, 2022

👍 That is consistent with the convention we use

@gallais
Copy link
Member Author

gallais commented Sep 14, 2022

I see that we also have map-++-commute instead of map-++.

@jamesmckinna
Copy link
Contributor

And similarly sum-++-commute.
I'll open a PR.

@jamesmckinna
Copy link
Contributor

What about the *-commute properties in Data.Sum.Properties?

@JacquesCarette
Copy link
Contributor

I wonder if map-++-commute might have been my doing. I like that name... but I'm completely fine with map-++ if that's the library convention.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Sep 16, 2022

@JacquesCarette I take your point about 'small' PRs: but #626 and #509 are about consistency/uniformity of names according to a systematic library convention (whose stipulation to a large extent is neither here nor there; consistency is the main consideration here); the case of map-id(entity) was, at least on my reading, of the same order of need for consistency, as that of map-∘ which started this issue for milestone 2.0...

I'm certainly open to the possibility that Codata.* properties might better want distinct names, to draw attention to the fact that

  • in the case of -identity, the relation is that of bismilarity, not equality
  • in the case of -∘, that the direction of the equation is opposite to that in Data.*

For this reason, I have stopped pushing did not yet push any commits regarding changes to Codata

@JacquesCarette
Copy link
Contributor

@jamesmckinna it is indeed a philosophical disagreement. You see this PR as achieving consistency/uniformity as a complete unit, while I'd be quite happy with a sequence of PRs whose union would achieve that. Moving closer to the goal is worthwhile, even if the ultimate goal (for whatever reason) is not ultimately achieve for 2.0. [I use this philosophy quite strongly in my Drasil project.]

Ultimately the person who is actually doing the work gets to decide, as this is all volunteer work. I believe in that 'philosophy' even more strongly than in the above!

@jamesmckinna
Copy link
Contributor

Thanks for clarification. Given our discussion on Monday about the 2.0 milestone, I was indeed keen to achieve 'completeness' in closing these two issues. But yes, experience with git(hub) has taught me that small(er) steps are usually better. But this one, 'bigger' as it may seem, has proved largely routine. Updating CHANGELOG is perhaps another matter.

@jamesmckinna
Copy link
Contributor

Updated CHANGELOG.

@jamesmckinna jamesmckinna linked a pull request Sep 16, 2022 that will close this issue
4 tasks
@jamesmckinna
Copy link
Contributor

jamesmckinna commented Sep 16, 2022

Outstanding instances of -commute:

  • Data.Sum.Properties affecting Data.Fin.Properties and Data.Vec.Properties through [,]-map-commute and [,]-∘-distr (!), plus a couple of other map-commute-like lemmas not used elsewhere
  • Data.Vec.Recursive.Properties with a single lemma append-cons-commute used only internally
  • Data.Tree.Binary.Zipper.Properties with a number of toTree-*-commute lemmas
  • Data.Integer.Properties affecting Data.Integer.Coprimality, Data.Integer.Divisibility.Signed, Data.Integer.Divisibility, Data.Integer.DivMod, and Data.Rational.Properties, through abs-*-commute and pos-+-commute

Only the first really seems worth tackling (not least for the mildly non-trivial knock-on viscosity); the second two are leaf modules in the library dependency graph (EDITED: and are therefore low-hanging-fruit, so I've knocked those off as well); and the last seems to require some prior work on #501 before proceeding. Suggestions as to how to proceed further?

jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
jamesmckinna added a commit to jamesmckinna/agda-stdlib that referenced this issue Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants