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

[Merged by Bors] - chore(*/pi): rename *_hom.apply to pi.eval_*_hom #5851

Closed
wants to merge 10 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Jan 22, 2021

These definitions state the fact that fixing an i and applying a function (Π i, f i) maintains the algebraic structure of the function. We already have a name for this operation, function.eval.

These isn't a statement about monoid_hom or ring_hom at all - that just happens to be their type.
For this reason, this commit moves all the definitions of this type into the pi namespace:

  • add_monoid_hom.applypi.eval_add_monoid_hom
  • monoid_hom.applypi.eval_monoid_hom
  • ring_hom.applypi.eval_ring_hom
  • pi.alg_hom.apply [sic] → pi.eval_alg_hom

This scales better, because we might want to say that applying a linear_map over a non-commutative ring is an add_monoid_hom. Using the naming convention established by this commit, that's easy; it's linear_map.eval_add_monoid_hom to mirror pi.apply_add_monoid_hom.

This partially addresses this zulip discussion


Split from #5834

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Jan 22, 2021
@eric-wieser eric-wieser added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Feb 1, 2021
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Feb 22, 2021
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label May 10, 2021
@eric-wieser eric-wieser changed the title chore(*/pi): rename *_hom.apply to pi.apply_*_hom chore(*/pi): rename *_hom.apply to pi.eval_*_hom May 10, 2021
@eric-wieser eric-wieser force-pushed the eric-wieser/rename-monoid_hom.apply branch from d5685d7 to 57630bb Compare May 10, 2021 12:24
@jcommelin
Copy link
Member

My only worry is that these names now become quite long. I've regularly used open add_monoid_hom to have nice and short homs like apply, eval, etc...

@eric-wieser
Copy link
Member Author

Let's see whether CI indicates that code like that has ended up in mathlib. It would certainly help if we had a shorter suffix than _add_monoid_hom to indicate that foo_add_monoid_hom is just foo bundled as an add_monoid_hom. I guess this is where the unbundled API wins out, because instead it becomes the prefix add_monoid_hom.of foo

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels May 12, 2021
@fpvandoorn
Copy link
Member

LGTM

the names didn't seem to get much longer, since (at least currently) the namespaces were not opened.

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels May 15, 2021
bors bot pushed a commit that referenced this pull request May 15, 2021
These definitions state the fact that fixing an `i` and applying a function `(Π i, f i)` maintains the algebraic structure of the function. We already have a name for this operation, `function.eval`.

These isn't a statement about `monoid_hom` or `ring_hom` at all - that just happens to be their type.
For this reason, this commit moves all the definitions of this type into the `pi` namespace:

* `add_monoid_hom.apply` → `pi.eval_add_monoid_hom`
* `monoid_hom.apply` → `pi.eval_monoid_hom`
* `ring_hom.apply` → `pi.eval_ring_hom`
* `pi.alg_hom.apply` [sic] → `pi.eval_alg_hom`

This scales better, because we might want to say that applying a `linear_map` over a non-commutative ring is an `add_monoid_hom`. Using the naming convention established by this commit, that's easy; it's `linear_map.eval_add_monoid_hom` to mirror `pi.apply_add_monoid_hom`.

This partially addresses [this zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/Naming.3A.20eval.20vs.20apply/near/223813950)
@bors
Copy link

bors bot commented May 16, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(*/pi): rename *_hom.apply to pi.eval_*_hom [Merged by Bors] - chore(*/pi): rename *_hom.apply to pi.eval_*_hom May 16, 2021
@bors bors bot closed this May 16, 2021
@bors bors bot deleted the eric-wieser/rename-monoid_hom.apply branch May 16, 2021 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants