Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

chore(category_theory): using simps#1500

Merged
mergify[bot] merged 6 commits into
masterfrom
category_theory_simps
Oct 18, 2019
Merged

chore(category_theory): using simps#1500
mergify[bot] merged 6 commits into
masterfrom
category_theory_simps

Conversation

@kim-em
Copy link
Copy Markdown
Collaborator

@kim-em kim-em commented Oct 1, 2019

Going through the category theory library and using @[simps].

I stopped partway through because I heard from @rwbarton that he'd also started to do this. I'll dump what I did here until we can coordinate.

@kim-em kim-em added the WIP Work in progress label Oct 1, 2019
@kim-em kim-em requested a review from a team as a code owner October 1, 2019 04:52
@khoek
Copy link
Copy Markdown
Collaborator

khoek commented Oct 1, 2019

It'll be great to see the -195 grow! To be honest, are there good examples when wouldn't you want lean to generate these lemmas for you when you make any def? (Could it hurt?)

Comment thread src/category_theory/limits/cones.lean Outdated
Comment thread src/category_theory/limits/cones.lean Outdated
Comment thread src/category_theory/limits/cones.lean Outdated
Comment thread src/category_theory/limits/cones.lean Outdated
Comment thread src/category_theory/monad/limits.lean Outdated
end }

@[simp] lemma cone_point_a (D : J ⥤ algebra T) [has_limit.{v₁} (D ⋙ forget T)] :
(cone_point D).a = limit.lift _ (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was there a reason the old lemma was stated in such a way, not referring to c?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not that I've been able to work out... My best guess is that I was struggling with something that turned out to not be an issue, but I never cleaned up at the end, and it slipped through the PR process.

Comment thread src/category_theory/whiskering.lean Outdated
Comment thread src/category_theory/whiskering.lean Outdated
@rwbarton
Copy link
Copy Markdown
Collaborator

rwbarton commented Oct 8, 2019

Did you have thoughts about the overlapping simp lemmas? My feeling is that I don't really know how bad they are, but the gain from using simps is not that big per occurrence, so I was more inclined not to use simps in those cases.

@kim-em kim-em added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Oct 14, 2019
@kim-em
Copy link
Copy Markdown
Collaborator Author

kim-em commented Oct 14, 2019

Okay, we may be good to go again, sorry about the delay. I've reverted all the overlapping simps.

Copy link
Copy Markdown
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.

Looks good to me!

Copy link
Copy Markdown
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

Looks good to me, too.

@fpvandoorn fpvandoorn 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 Oct 18, 2019
@mergify mergify Bot merged commit 05102ec into master Oct 18, 2019
@mergify mergify Bot deleted the category_theory_simps branch October 18, 2019 19:42
butterthebuddha pushed a commit to butterthebuddha/mathlib that referenced this pull request May 15, 2020
* chore(category_theory): using simps

* more simps

* remove simp lemma

* revertings overlapping @[simps]
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

5 participants