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] - chore(Algebra/TrivSqZeroExt): use open scoped RightActions #10546

Closed
wants to merge 6 commits into from

Conversation

eric-wieser
Copy link
Member

One lemma statement has changed (up to associativity).

This is one of the more compelling justifications for #8909.


Open in Gitpod

One lemma statement has changed (up to associativity)
@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR awaiting-CI t-algebra Algebra (groups, rings, fields etc) labels Feb 14, 2024
Copy link
Collaborator

@j-loreaux j-loreaux left a comment

Choose a reason for hiding this comment

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

This is indeed much nicer. I went through the file and saw a few places where I would prefer to change to •> to aid the eye. I'm only interested in it in places where there is an op smul. Personally, I would like to prefer that as the default style (i.e., if a SMul Rᵐᵒᵖ M instance is in context, we write •> instead of ).

Because GitHub doesn't allow me to easily reference things not in the diff, I'll just list the line numbers: 526, 578, 624, 636, 646, 769.

@j-loreaux j-loreaux added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Feb 15, 2024
@eric-wieser
Copy link
Member Author

Because GitHub doesn't allow me to easily reference things not in the diff, I'll just list the line numbers:

  1. show x.1 • (0 : M) + (1 : Rᵐᵒᵖ) • x.2 = x.2 by rw [smul_zero, zero_add, one_smul] }

    Slightly awkward because I have to then use op_one in the proof before I can use one_smul

  2. show x.1 • (0 : M) + (0 : Rᵐᵒᵖ) • x.2 = 0 by rw [smul_zero, zero_add, zero_smul]

    Same, but for 0

  3. (h : x.snd <• x.fst = x.fst •> x.snd) : snd (x ^ n) = n • x.fst ^ n.pred • x.snd := by

    refine' (List.sum_eq_card_nsmul _ (x.fst ^ n • x.snd) _).trans _

    In this case the point of the lemma is that the two actions are in this case the same; but maybe that's not so good an argument.

  4. [IsCentralScalar R M] (x : tsze R M) (n : ℕ) : snd (x ^ n) = n • x.fst ^ n.pred • x.snd :=

    I think the argument above applies more strongly here; if you have IsCentralScalar you never need to make the distinction

  5. show r • x.snd = algebraMap S R r •> x.snd + (0 : M) <• x.fst by

    This is an action by S not R, which has no right action clearly I should change the variable name

@eric-wieser
Copy link
Member Author

(i.e., if a SMul Rᵐᵒᵖ M instance is in context, we write •> instead of ).

It would be neat if we could teach the pretty-printer this rule.

@eric-wieser
Copy link
Member Author

eric-wieser commented Feb 15, 2024

I've addressed 1 and 2 above, and made 5 moot by renaming. I've solved my objection to 3 by adding a second lemma stated with right actions, and I claim 4 needs no change.

@eric-wieser eric-wieser added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Feb 16, 2024
@j-loreaux
Copy link
Collaborator

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 Feb 17, 2024
mathlib-bors bot pushed a commit that referenced this pull request Feb 17, 2024
One lemma statement has changed (up to associativity).

This is one of the more compelling justifications for #8909.
@mathlib-bors
Copy link

mathlib-bors bot commented Feb 17, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Algebra/TrivSqZeroExt): use open scoped RightActions [Merged by Bors] - chore(Algebra/TrivSqZeroExt): use open scoped RightActions Feb 17, 2024
@mathlib-bors mathlib-bors bot closed this Feb 17, 2024
@mathlib-bors mathlib-bors bot deleted the eric-wieser/use-rightaction-notation branch February 17, 2024 03:41
riccardobrasca pushed a commit that referenced this pull request Feb 18, 2024
One lemma statement has changed (up to associativity).

This is one of the more compelling justifications for #8909.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
One lemma statement has changed (up to associativity).

This is one of the more compelling justifications for #8909.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

2 participants