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: introduce NthRoot notation class #7907

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

feat: introduce NthRoot notation class #7907

wants to merge 22 commits into from

Conversation

urkud
Copy link
Member

@urkud urkud commented Oct 25, 2023

Also use it for Real.sqrt, and replace the few occurrences of sqrt that do not already use the notation.


Zulip

Open in Gitpod

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Oct 30, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Feb 13, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Feb 14, 2024
@fpvandoorn fpvandoorn added the awaiting-author A reviewer has asked the author a question or requested changes label Apr 5, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 10, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 11, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
mathlib-bors bot pushed a commit that referenced this pull request Apr 13, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 13, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 13, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 14, 2024
Louddy pushed a commit that referenced this pull request Apr 15, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
callesonne pushed a commit that referenced this pull request Apr 22, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Jun2M pushed a commit that referenced this pull request Apr 24, 2024
This adds the notation `√r` for `Real.sqrt r`. The precedence is such that `√x⁻¹` is parsed as `√(x⁻¹)`; not because this is particularly desirable, but because it's the default and the choice doesn't really matter.

This is extracted from #7907, which adds a more general nth root typeclass.
The idea is to perform all the boring substitutions downstream quickly, so that we can play around with custom elaborators with a much slower rate of code-rot.
This PR also won't rot as quickly, as it does not forbid writing `x.sqrt` as that PR does.

While perhaps claiming `√` for `Real.sqrt` is greedy; it:
* Is far more common thatn `NNReal.sqrt` and `Nat.sqrt`
* Is far more interesting to mathlib than `sqrt` on `Float`
* Can be overloaded anyway, so this does not prevent downstream code using the notation on their own types.
* Will be replaced by a more general typeclass in a future PR.

[Zulip](https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/.60Sqrt.60.20notation.20typeclass/near/400086502)



Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label May 25, 2024
@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR awaiting-CI and removed awaiting-author A reviewer has asked the author a question or requested changes labels May 25, 2024
@eric-wieser eric-wieser requested a review from kmill May 25, 2024 21:47
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

I pushed some more changes, this now looks great.

@kmill, could you check the unexpander / use of default_instance is reasonable?

Comment on lines +40 to +48
@[app_unexpander NthRoot.nthRoot]
def NthRoot.unexpander : Lean.PrettyPrinter.Unexpander
| `($_ $n $a) =>
match n with
| `(2) => `(√$a)
| `(3) => `(∛$a)
| `(4) => `(∜$a)
| _ => `($n:superscript √$a)
| _ => throw ()
Copy link
Contributor

Choose a reason for hiding this comment

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

If the superscript is complicated (even with just NthRoot.nthRoot (2*n) x) it won't pretty print correctly. I think restricting to just number literals and identifiers should handle the common cases:

Suggested change
@[app_unexpander NthRoot.nthRoot]
def NthRoot.unexpander : Lean.PrettyPrinter.Unexpander
| `($_ $n $a) =>
match n with
| `(2) => `(√$a)
| `(3) => `(∛$a)
| `(4) => `(∜$a)
| _ => `($n:superscript √$a)
| _ => throw ()
@[app_unexpander NthRoot.nthRoot]
def NthRoot.unexpander : Lean.PrettyPrinter.Unexpander
| `($_ 2 $a) => `(√$a)
| `($_ 3 $a) => `(∛$a)
| `($_ 4 $a) => `(∜$a)
| `($_ $n:num $a) | `($_ $n:ident $a) => `($n:superscript √$a)
| _ => throw ()

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-CI awaiting-review The author would like community review of the PR merge-conflict The PR has a merge conflict with master, and needs manual merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants