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

feat(extensionality): rename to ext; generate ext rules for structures #1645

Merged
merged 11 commits into from
Nov 7, 2019

Conversation

cipher1024
Copy link
Collaborator

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • for tactics:
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

@semorrison
Copy link
Collaborator

What does this do?

@semorrison
Copy link
Collaborator

(Perhaps we should update the PR template so it prominently asks for a summary, before the checklist.)

@digama0
Copy link
Member

digama0 commented Nov 4, 2019

@semorrison Tell me I'm crazy, but didn't you use a tactic like this one in the category theory library? See https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/Equivalent.20of.20subtype.2Eeq.20for.20structures.3F/near/179768896

@digama0
Copy link
Member

digama0 commented Nov 4, 2019

@cipher1024 Why the name change in this tactic? This is just @[extensionality] theorem T.ext, automatically derived. I would prefer the name @[derive extensionality] if possible.

@semorrison
Copy link
Collaborator

Nope, I'd asked for it, but never got around to writing it. Good things come to those who wait! (Well, not usually.)

@robertylewis
Copy link
Member

@cipher1024 Why the name change in this tactic? This is just @[extensionality] theorem T.ext, automatically derived. I would prefer the name @[derive extensionality] if possible.

This was my gut response too, but right now derive is used specifically for type class instances. (Also the interface for a derive handler is slightly different.) Here's a half-baked idea: what about modifying @[extensionality] itself, so it can be applied either to a lemma like now, or to a structure, in which case it generates the relevant lemma?

@robertylewis
Copy link
Member

Independent of the above: are we consistent in the library about using type.ext and type.eq? I just checked subtype, which uses the first for the iff version and the second for the -> version. Maybe this tactic could generate both. It should apply the extensionality attribute to the proper one, which I think is the -> one?

@jcommelin
Copy link
Member

I think there are also a lot of ext and ext_iff pairs in the library.

@digama0
Copy link
Member

digama0 commented Nov 4, 2019

subtype.eq is different from the convention, but it's in core so 🤷‍♂️ .

@cipher1024
Copy link
Collaborator Author

I didn't think of it. I'll use .ext. Also, I like the idea of merging this with ext.

@semorrison
Copy link
Collaborator

semorrison commented Nov 4, 2019 via email

@cipher1024
Copy link
Collaborator Author

I agree

@bryangingechen
Copy link
Collaborator

Would it make sense to let [extensionality] work as well, for backwards compatibility?

@cipher1024
Copy link
Collaborator Author

I don't think that's a good idea. The change is very easy to make and having a synonym for an attribute is going to be messy and ugly.

@robertylewis robertylewis changed the title feat(tactic/eq_proof_rule): generate an equality proof rule for structures feat(extensionality): rename to ext; generate ext rules for structures Nov 5, 2019
Copy link
Member

@robertylewis robertylewis 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! Could you update the doc string for the ext attribute and its entry in tactics.md?

src/tactic/ext.lean Outdated Show resolved Hide resolved
src/tactic/ext.lean Outdated Show resolved Hide resolved
src/tactic/ext.lean Outdated Show resolved Hide resolved
cipher1024 and others added 5 commits November 5, 2019 07:48
Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>
Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>
Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>
Copy link
Member

@robertylewis robertylewis 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! I think there are two related tasks remaining: replacing manually defined ext lemmas with the attribute, and cleaning up naming issues. (We should uniformly use ext, ext_iff, eq, etc.) I'll make an issue for this, it's material for another PR.

@robertylewis robertylewis added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Nov 6, 2019
@bryangingechen
Copy link
Collaborator

Looks like [skip ci] on the last commit message keeps mergify from doing its thing. Did a merge to get this rolling again.

@mergify mergify bot merged commit c718a22 into master Nov 7, 2019
@mergify mergify bot deleted the struct_eq_rule branch November 7, 2019 03:44
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…tures (leanprover-community#1645)

* Update core.lean

* Update tactics.lean

* integrate generation of extensionality lemma of structures into `ext`

* Update src/tactic/ext.lean [skip ci]

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update src/tactic/ext.lean [skip ci]

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update src/tactic/ext.lean

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update ext.lean [skip ci]

* Update tactics.md [skip ci]

* fix build

* fix build
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 16, 2020
…tures (leanprover-community#1645)

* Update core.lean

* Update tactics.lean

* integrate generation of extensionality lemma of structures into `ext`

* Update src/tactic/ext.lean [skip ci]

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update src/tactic/ext.lean [skip ci]

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update src/tactic/ext.lean

Co-Authored-By: Rob Lewis <Rob.y.lewis@gmail.com>

* Update ext.lean [skip ci]

* Update tactics.md [skip ci]

* fix build

* fix build
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

6 participants