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

chore(category_theory/limits): unify naming of lemmas related to the (co)lim functor #2040

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

TwoFX
Copy link
Member

@TwoFX TwoFX commented Feb 22, 2020

lim.map_π was the only lemma of the form lim.* rather than limit.*, and I do not see any way it differs from limit.map_pre and others that could justify the different naming.

@@ -616,16 +616,16 @@ def lim : (J ⥤ C) ⥤ C :=

variables {F} {G : J ⥤ C} (α : F ⟶ G)

@[simp, reassoc] lemma lim.map_π (j : J) : lim.map α ≫ limit.π G j = limit.π F j ≫ α.app j :=
@[simp, reassoc] lemma limit.map_π (j : J) : lim.map α ≫ limit.π G j = limit.π F j ≫ α.app j :=
Copy link
Member

Choose a reason for hiding this comment

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

I think the reason it was called lim.map_pi is that the statement starts with lim.map \alpha .... Hence it follows the naming conventions, and I'm inclined to leave it the way it was...

Copy link
Member Author

@TwoFX TwoFX Feb 22, 2020

Choose a reason for hiding this comment

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

This makes sense, but I feel that if the name lim.map_pi is used, then the name lim.map_pre should also be used instead of limit.map_pre. On the other hand, limit.map_pre' would still be correct, so a series of very closely related lemmas would have different prefixes.

As someone who tried to use these lemmas, I found the current situation to be very confusing, and I chose this way of unification because it looked like it would require the fewest changes, but I'd be happy to change the PR to instead rename limit.map_pre (and maybe limit.map_post?).

Copy link
Member

Choose a reason for hiding this comment

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

@TwoFX Fair enough, I agree with the confusion. I think that limit.map_pre is actually the one that is "violating" the naming convention. If you want to PR those changes, that would be nice.

@TwoFX
Copy link
Member Author

TwoFX commented Feb 22, 2020

@jcommelin I have adapted the names of the lemmas as discussed. Note that in this version the naming is not consistent across duals. For example, we have lim.map_π, but its dual is called colimit.ι_map. (I am not trying to say that this is a bad thing. I just want to draw attention to it in case it is.)

@jcommelin
Copy link
Member

Hmmm... I agree that that is also confusing. I think being consistent across duals is maybe more important than other considerations. @semorrison do you have some helpful advice?

@semorrison
Copy link
Collaborator

Let's just name everything with limit. or colimit.. I think uniformity is better than any of the non-uniform rules, all of which have some surprise factor.

(The underlying confusion here is that I decided to use the shortened lim and colim for the "limits packaged as a functor" gadgets. Another alternative would be to work out if we can rename those.)

So --- @TwoFX, would you mind restoring your PR to the original state?

@semorrison semorrison added the awaiting-author A reviewer has asked the author a question or requested changes label Feb 25, 2020
@TwoFX
Copy link
Member Author

TwoFX commented Feb 25, 2020

So --- @TwoFX, would you mind restoring your PR to the original state?

Done.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Cool, I agree with Scott, after second thought. It's on the merge queue.

@jcommelin jcommelin 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-author A reviewer has asked the author a question or requested changes labels Feb 25, 2020
@mergify mergify bot merged commit a227e06 into leanprover-community:master Feb 25, 2020
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…munity#2040)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…munity#2040)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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