Skip to content

[Merged by Bors] - feat(Algebra/Category/ModuleCat/Basic): remove duplicate ofHom#17476

Closed
mo271 wants to merge 1 commit intomasterfrom
mo271/rm_ofHom
Closed

[Merged by Bors] - feat(Algebra/Category/ModuleCat/Basic): remove duplicate ofHom#17476
mo271 wants to merge 1 commit intomasterfrom
mo271/rm_ofHom

Conversation

@mo271
Copy link
Copy Markdown
Collaborator

@mo271 mo271 commented Oct 6, 2024

We keep asHom and use that throughout, since there seems to be no need to keep both around.


Open in Gitpod

We keep `asHom` and use that throughout, since there seems to be no
need to keep both around.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 6, 2024

PR summary ed9185159b

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ ModuleCat.asHom_apply
+ ModuleCat.ofHom
+ ModuleCat.ofHom_apply
- ofHom
- ofHom_apply

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks!
maintainer merge

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 8, 2024

🚀 Pull request has been placed on the maintainer queue by grunweg.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Oct 8, 2024
@kbuzzard
Copy link
Copy Markdown
Member

kbuzzard commented Oct 8, 2024

Thanks!

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Oct 8, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 8, 2024
)

We keep `asHom` and use that throughout, since there seems to be no need to keep both around.




Co-authored-by: Moritz Firsching <firsching@google.com>
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Oct 8, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Algebra/Category/ModuleCat/Basic): remove duplicate ofHom [Merged by Bors] - feat(Algebra/Category/ModuleCat/Basic): remove duplicate ofHom Oct 8, 2024
@mathlib-bors mathlib-bors bot closed this Oct 8, 2024
@mathlib-bors mathlib-bors bot deleted the mo271/rm_ofHom branch October 8, 2024 09:15
mathlib-bors bot pushed a commit that referenced this pull request Dec 3, 2024
…eCat.ofHom` (#19705)

As discussed in the comments for #19511.

It seems the choice between `ofHom` and `asHom` is somewhat inconsistent in concrete categories, but `ofHom` wins. In particular, we want to be consistent with the newly refactored `AlgebraCat.ofHom` and the constructor for objects `ModuleCat.of`.

The choice between `asHom` and `ofHom` was only made a few months ago: #17476. It's a bit unfortunate to go back on something that was deprecated so recently, but I think the result is worth the pain.

I have not touched `asHomLeft` and `asHomRight` since #19511 will replace these with `ofHom` everywhere anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants