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] - feat(tactic/monotonicity): Allow @[mono] on strict_mono lemmas #7017

Closed
wants to merge 3 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Apr 3, 2021

A follow-up to #3310


Open in Gitpod

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Apr 3, 2021
@eric-wieser eric-wieser added the t-meta Tactics, attributes or user commands label Apr 4, 2021
@fpvandoorn
Copy link
Member

Can you add a test to the test folder to show that this works as expected?

@fpvandoorn fpvandoorn 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 Apr 5, 2021
@eric-wieser
Copy link
Member Author

eric-wieser commented Apr 5, 2021

I added a test, but all it checks is that @[mono] no longer rejects strict_mono. I can't work out how the mono tactic is supposed to be used with monotone and nor is that tested, so I can't add a test that actually exercises strict_mono.

edit: oh, we have monotonicity.lean and monotonicity/test_cases.lean which both test this tactic.

@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 Apr 5, 2021
@fpvandoorn
Copy link
Member

I can't work out how the mono tactic is supposed to be used with monotone and nor is that tested, so I can't add a test that actually exercises strict_mono.

I think that is tested here: https://github.com/leanprover-community/mathlib/blob/coe_minmax/test/monotonicity.lean#L114-L152
The mono tactic should at least work with strict_mono lemmas for this PR to be merged, otherwise this PR is useless. I don't know how hard-coded monotone is in the mono tactic, but maybe the tactic needs to change to accept strict_mono assumptions.

@fpvandoorn fpvandoorn 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 Apr 5, 2021
@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 Apr 5, 2021
@eric-wieser
Copy link
Member Author

Resolved - there were two different test files that seem not to be aware of each other, and I found the one that didn't have any hints of how to write the tests.

@semorrison
Copy link
Collaborator

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 Apr 6, 2021
bors bot pushed a commit that referenced this pull request Apr 6, 2021
@bors
Copy link

bors bot commented Apr 7, 2021

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Apr 7, 2021
@bors
Copy link

bors bot commented Apr 7, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(tactic/monotonicity): Allow @[mono] on strict_mono lemmas [Merged by Bors] - feat(tactic/monotonicity): Allow @[mono] on strict_mono lemmas Apr 7, 2021
@bors bors bot closed this Apr 7, 2021
@bors bors bot deleted the eric-wieser/strict_monotone branch April 7, 2021 10:30
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+`.) t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants