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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - fix: Fix Polynomial.repr #3442

Closed
wants to merge 7 commits into from
Closed

Conversation

Komyyy
Copy link
Collaborator

@Komyyy Komyyy commented Apr 15, 2023

I misunderstood the Repr instance when I port this file, In Lean 4, Repr returns Format, not String, so my porting of Polynomial.repr was unnatural.


I fixed that in this PR. Of course, Polynomial is noncomputable and this instance is actually useless. 馃ゴ

Open in Gitpod

@Komyyy Komyyy added awaiting-review The author would like community review of the PR mathlib-port This is a port of a theory file from mathlib. labels Apr 15, 2023
@eric-wieser
Copy link
Member

eric-wieser commented Apr 15, 2023

Can you add some tests for this? You can get computable output with

#eval (鉄ㄢ煥{0, 2}, Pi.single 0 11 + Pi.single 2 22, sorry鉄┾煩 : 鈩昜X])

@Komyyy
Copy link
Collaborator Author

Komyyy commented Apr 15, 2023

This works well.

C 11 + C 22 * X ^ 2

@jcommelin
Copy link
Member

Thanks 馃帀

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 20, 2023
@eric-wieser
Copy link
Member

This works well.

C 11 + C 22 * X ^ 2

I mean actually put something in the test directory! But that can be done in a follow-up.

bors bot pushed a commit that referenced this pull request Apr 20, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
@eric-wieser
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Apr 20, 2023

Canceled.

@Parcly-Taxel Parcly-Taxel removed the ready-to-merge This PR has been sent to bors. label Apr 20, 2023
test/Polynomial.lean Outdated Show resolved Hide resolved
Co-authored-by: Johan Commelin <johan@commelin.net>
@Komyyy Komyyy added the awaiting-review The author would like community review of the PR label Apr 22, 2023
@semorrison
Copy link
Contributor

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Apr 22, 2023
bors bot pushed a commit that referenced this pull request Apr 22, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
@bors
Copy link

bors bot commented Apr 22, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix: Fix Polynomial.repr [Merged by Bors] - fix: Fix Polynomial.repr Apr 22, 2023
@bors bors bot closed this Apr 22, 2023
@bors bors bot deleted the Polynomial.repr branch April 22, 2023 02:47
kbuzzard pushed a commit that referenced this pull request Apr 22, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
semorrison pushed a commit that referenced this pull request May 10, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
hrmacbeth pushed a commit that referenced this pull request May 10, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
hrmacbeth pushed a commit that referenced this pull request May 11, 2023
I misunderstood the `Repr` instance when I port this file, In Lean 4, `Repr` returns `Format`, not `String`, so my porting of `Polynomial.repr` was unnatural.



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Pol_tta <52843868+Komyyy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants