Skip to content

[Merged by Bors] - fix(Algebra/IndicatorFunction): minor fixes#6640

Closed
urkud wants to merge 2 commits intomasterfrom
YK-indicator-review
Closed

[Merged by Bors] - fix(Algebra/IndicatorFunction): minor fixes#6640
urkud wants to merge 2 commits intomasterfrom
YK-indicator-review

Conversation

@urkud
Copy link
Copy Markdown
Member

@urkud urkud commented Aug 17, 2023

  • Fix name in docstrings.
  • Use Type* in variable, explicitly use Sort*.
  • Use to_additive to generate the additive version.
  • Drop pattern matching.
  • Add Set.mulIndicator_diff',
    the multiplicative version of Set.indicator_diff.

Open in Gitpod

- Fix name in docstrings.
- Use `Type*` in `variable`, explicitly use `Sort*`.
- Use `to_additive` to generate the additive version.
- Drop pattern matching.
- Add `Set.mulIndicator_diff'`,
  the multiplicative version of `Set.indicator_diff`.
@urkud urkud added awaiting-review t-algebra Algebra (groups, rings, fields, etc) labels Aug 17, 2023
open Function

variable {α β ι M N : Type _}
variable {α β ι M N : Type*}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm surprised it even worked with the latest changes!

Comment on lines +576 to +577
@[to_additive indicator_diff]
theorem mulIndicator_diff' (h : s ⊆ t) (f : α → G) :
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you do the same for compl?
Also, it's a bit annoying if the primes in the name are mixed up (IIUC with your change we would have mulIndicator_diff <-> indicator_diff' and mulIndicator_diff' <-> indicator_diff), should we make sure that they match up (in a follow up PR since that's probably going to touch a lot of files) or is it expected that the "default" lemma is different?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It seems that we mostly use a * b⁻¹ for groups, a / b for fields, and a - b for additive groups.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay that makes sense. Let's keep it like that then.

Copy link
Copy Markdown
Member

@ADedecker ADedecker left a comment

Choose a reason for hiding this comment

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

Thanks!

bors d+

@bors
Copy link
Copy Markdown

bors bot commented Aug 17, 2023

✌️ urkud can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Aug 17, 2023
@urkud
Copy link
Copy Markdown
Member Author

urkud commented Aug 18, 2023

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 18, 2023
bors bot pushed a commit that referenced this pull request Aug 18, 2023
- Fix name in docstrings.
- Use `Type*` in `variable`, explicitly use `Sort*`.
- Use `to_additive` to generate the additive version.
- Drop pattern matching.
- Add `Set.mulIndicator_diff'`,
  the multiplicative version of `Set.indicator_diff`.
@bors
Copy link
Copy Markdown

bors bot commented Aug 19, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title fix(Algebra/IndicatorFunction): minor fixes [Merged by Bors] - fix(Algebra/IndicatorFunction): minor fixes Aug 19, 2023
@bors bors bot closed this Aug 19, 2023
@bors bors bot deleted the YK-indicator-review branch August 19, 2023 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants