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: port fin_cases #437

Closed
wants to merge 44 commits into from

Conversation

semorrison
Copy link
Contributor

This is a port of fin_cases. It doesn't yet support the with or using clauses from mathlib3.

I could remove the dependence on #433, but would have to (temporarily) remove all the tests to do so.

@semorrison semorrison added awaiting-review The author would like community review of the PR and removed blocked-by-other-PR This PR depends on another PR which is still in the queue. labels Oct 8, 2022
@digama0
Copy link
Member

digama0 commented Oct 18, 2022

I'm not enthused about this implementation, it has a lot of potential failure modes, but since it's not passing all the tests or implementing all the options we'll need to extend it later anyway, and this is good enough for now.

bors r+

bors bot pushed a commit that referenced this pull request Oct 18, 2022
This is a port of `fin_cases`. It doesn't yet support the `with` or `using` clauses from mathlib3.

I could remove the dependence on #433, but would have to (temporarily) remove all the tests to do so.

* depends on: #433

Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link

bors bot commented Oct 18, 2022

Build failed:

@semorrison
Copy link
Contributor Author

The implementation is pretty much isomorphic to the one in mathlib3, so I'll blame the author of that. (Err... sorry.)

After I merge master it is all failing, with metavariables in the declarations, and I'm not immediately seeing why.

@semorrison
Copy link
Contributor Author

Ugh, voodoo changing a 3 to a 2 fixes the main problem.

Currently, if you call fin_cases h on a bad hypotheses that it can't handle, it just deletes the goal and reports success. Not cool...

@semorrison
Copy link
Contributor Author

Okay, at least superficially fixed that, now we don't catch the error if cases outright fails.

@semorrison
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Oct 20, 2022
This is a port of `fin_cases`. It doesn't yet support the `with` or `using` clauses from mathlib3.

I could remove the dependence on #433, but would have to (temporarily) remove all the tests to do so.

* depends on: #433

Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link

bors bot commented Oct 20, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat: port fin_cases [Merged by Bors] - feat: port fin_cases Oct 20, 2022
@bors bors bot closed this Oct 20, 2022
@bors bors bot deleted the semorrison/fin_cases branch October 20, 2022 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants