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: Finset builder notation #11582

Closed
wants to merge 9 commits into from

Conversation

YaelDillies
Copy link
Collaborator

@YaelDillies YaelDillies commented Mar 22, 2024

Define elaborators (but no delaborators) to elaborate the following notations to a Finset:

In Data.Finset.Basic,

  • {x ∈ s | p x}

In Data.Fintype.Basic,

  • {x | p x}
  • {x : α | p x}
  • {x ∉ s | p x}
  • {x ≠ a | p x}

In Order.Interval.Finset.Basic,

  • {x ≤ a | p x}
  • {x ≥ a | p x}
  • {x < a | p x}
  • {x > a | p x}

The general heuristic for deciding whether to elaborate a given notation as a Set or Finset is:

  • Check whether the expected type is Finset ?α.
    • If it is, elaborate as a Finset.
    • If it isn't, check whether the expected type of s in the notation is Finset ?α.
      • If it is, elaborate as a Finset.
      • If it isn't or there is no s in the notation, elaborate as a Set.

There is currently a gotcha that elaboration to Set is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See Zulip.


Open in Gitpod

@YaelDillies YaelDillies added WIP Work in progress t-meta Tactics, attributes or user commands labels Mar 22, 2024
@YaelDillies YaelDillies force-pushed the finset_builder branch 2 times, most recently from 777235c to d9f35cb Compare March 25, 2024 06:34
@YaelDillies YaelDillies added awaiting-review and removed WIP Work in progress labels Mar 25, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Mar 26, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Apr 21, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels May 7, 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. (this label is managed by a bot) label May 27, 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. (this label is managed by a bot) label May 27, 2024
@YaelDillies YaelDillies changed the title Feat: Finset builder notation feat: Finset builder notation May 27, 2024
@Vierkantor Vierkantor added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels May 29, 2024
@YaelDillies
Copy link
Collaborator Author

The new notations here intersect a lot with the notations introduced by #6795, so I'm thinking I should be creating a new syntax category setBinder instead of reusing the existing binderPred.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label May 30, 2024
Copy link

github-actions bot commented Jun 11, 2024

PR summary 985d7de269

Import changes for modified files

Dependency changes

File Base Count Head Count Change
Mathlib.Init.Set 1 2 +1 (+100.00%)
Import changes for all files
Files Import difference
Mathlib.Init.Set 1

Declarations diff

+ elabFinsetBuilderIxx
+ elabFinsetBuilderSep
+ elabFinsetBuilderSetOf
+ elabSetBuilder
+ knownToBeFinsetNotSet

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Jun 11, 2024
@YaelDillies
Copy link
Collaborator Author

The new notations here intersect a lot with the notations introduced by #6795, so I'm thinking I should be creating a new syntax category setBinder instead of reusing the existing binderPred.

Given that this is completely orthogonal to the current PR, I will do that in a later PR.

@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes label Jun 11, 2024
Copy link
Collaborator

@joneugster joneugster left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Generally looks clean to me. Here are some remaining comments

test/finset_builder.lean Show resolved Hide resolved
Mathlib/Data/Fintype/Basic.lean Show resolved Hide resolved
Mathlib/Data/Fintype/Basic.lean Outdated Show resolved Hide resolved
@joneugster joneugster self-assigned this Aug 13, 2024
@joneugster joneugster added the awaiting-author A reviewer has asked the author a question or requested changes label Aug 13, 2024
@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes label Aug 14, 2024
@joneugster joneugster added the awaiting-author A reviewer has asked the author a question or requested changes label Aug 15, 2024
Co-authored-by: Jon Eugster <eugster.jon@gmail.com>
@YaelDillies YaelDillies removed the awaiting-author A reviewer has asked the author a question or requested changes label Aug 15, 2024
@joneugster
Copy link
Collaborator

maintainer merge

Copy link

🚀 Pull request has been placed on the maintainer queue by joneugster.

@kmill
Copy link
Contributor

kmill commented Aug 15, 2024

Thanks!

bors r+

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 15, 2024
-- If the expected type is known to be `Set ?α`, give up. If it is not known to be `Set ?α` or
-- `Finset ?α`, check the expected type of `s`.
unless ← knownToBeFinsetNotSet expectedType? do
let ty ← try whnfR (← inferType (← elabTerm s none)) catch _ => throwUnsupportedSyntax
Copy link
Contributor

Choose a reason for hiding this comment

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

It's too bad that this elabTerm s none is wasted work. Maybe in a followup it could be saved and then inserted into the Syntax on line 2201 using exprToSyntax.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that makes sense

mathlib-bors bot pushed a commit that referenced this pull request Aug 15, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Aug 15, 2024

Build failed (retrying...):

mathlib-bors bot pushed a commit that referenced this pull request Aug 16, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Aug 16, 2024

This PR was included in a batch that was canceled, it will be automatically retried

mathlib-bors bot pushed a commit that referenced this pull request Aug 16, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Aug 16, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: Finset builder notation [Merged by Bors] - feat: Finset builder notation Aug 16, 2024
@mathlib-bors mathlib-bors bot closed this Aug 16, 2024
@mathlib-bors mathlib-bors bot deleted the finset_builder branch August 16, 2024 01:47
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
Define elaborators (but no delaborators) to elaborate the following notations to a `Finset`:

In `Data.Finset.Basic`,
* `{x ∈ s | p x}`

In `Data.Fintype.Basic`,
* `{x | p x}`
* `{x : α | p x}`
* `{x ∉ s | p x}`
* `{x ≠ a | p x}`

In `Order.Interval.Finset.Basic`,
* `{x ≤ a | p x}`
* `{x ≥ a | p x}`
* `{x < a | p x}`
* `{x > a | p x}`

The general heuristic for deciding whether to elaborate a given notation as a `Set` or `Finset` is:
* Check whether the expected type is `Finset ?α`.
  * If it is, elaborate as a `Finset`.
  * If it isn't, check whether the expected type of `s` in the notation is  `Finset ?α`.
    * If it is, elaborate as a `Finset`.
    * If it isn't or there is no `s` in the notation, elaborate as a `Set`.

There is currently a gotcha that elaboration to `Set` is highly preferred because we can't postpone metavariable and override the set elaborator without breaking many existing uses of set builder notation. See [Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/.7Bx.20.E2.88.88.20s.20.7C.20p.20x.7D.20notation.20for.20finset).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants