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(measure_theory/integral): Circle integral transform #13885

Closed
wants to merge 40 commits into from

Conversation

CBirkbeck
Copy link
Collaborator

@CBirkbeck CBirkbeck commented May 2, 2022

Some basic definitions and results related to circle integrals of a function. These form part of #13500


Open in Gitpod

@CBirkbeck CBirkbeck marked this pull request as ready for review May 2, 2022 15:04
@CBirkbeck CBirkbeck added the awaiting-review The author would like community review of the PR label May 2, 2022
@jcommelin jcommelin changed the title Circle integral transform feat(measure_theory/intregral): Circle integral transform May 4, 2022
@loefflerd loefflerd changed the title feat(measure_theory/intregral): Circle integral transform feat(measure_theory/integral): Circle integral transform May 6, 2022
Copy link
Collaborator

@loefflerd loefflerd left a comment

Choose a reason for hiding this comment

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

Some cosmetic code-style fixes

src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
@CBirkbeck
Copy link
Collaborator Author

Some cosmetic code-style fixes

Done. Thank you!

@sgouezel
Copy link
Collaborator

sgouezel commented May 6, 2022

Can you merge master to fix the build? (The linting problem is not your fault, the solution is to merge master where it has been solved).

Copy link
Collaborator

@loefflerd loefflerd left a comment

Choose a reason for hiding this comment

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

More stylistic nitpicking

src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
src/measure_theory/integral/circle_integral_transform.lean Outdated Show resolved Hide resolved
@loefflerd
Copy link
Collaborator

I golfed it a bit, and rearranged the code to make things slightly more readable. The only change to functionality is that I made a few arguments to lemmas implicit (curly braces rather than brackets).

@loefflerd
Copy link
Collaborator

Here are some comments of a little more mathematical substance.

  • How do you distinguish between what goes in circle_integral.lean and what goes in circle_integral_transform.lean? In particular the last few lemmas of the existing file explicitly involve the Cauchy integral formula, so the distinction between the two files seems a bit unclear to me.
  • There is a lot of juggling between different versions of intervals -- we have [0, 2 * π], interval 0 (2 * π), and Ι 0 (2 * π) (the last being particularly unfortunate for readability since the Greek capital Iota is visually identical to roman I). May I suggest standardizing all statements to [0, 2 * π]? If you need to call lemmas from other files which expect a half-open interval, you can just use an Ioc_subset_Icc_self inline, rather than making a separate lemma for it.

@CBirkbeck
Copy link
Collaborator Author

CBirkbeck commented May 9, 2022

There is a lot of juggling between different versions of intervals -- we have [0, 2 * π], interval 0 (2 * π), and Ι 0 (2 * π) (the last being particularly unfortunate for readability since the Greek capital Iota is visually identical to roman I). May I suggest standardizing all statements to [0, 2 * π]? If you need to call lemmas from other files which expect a half-open interval, you can just use an Ioc_subset_Icc_self inline, rather than making a separate lemma for it.

This is a good point. I've removed some of the lemmas that are now redundant.

@CBirkbeck
Copy link
Collaborator Author

CBirkbeck commented May 9, 2022

How do you distinguish between what goes in circle_integral.lean and what goes in circle_integral_transform.lean? In particular the last few lemmas of the existing file explicitly involve the Cauchy integral formula, so the distinction between the two files seems a bit unclear to me.

I really don't know how to distinguish. I just thought it would be easier having it in a separate file, but if its preferable I could just put this in a section of circle_integral. But once the rest of the #13500 is added it might be a bit long...

@bors
Copy link

bors bot commented Jul 5, 2022

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

@leanprover-community-bot-assistant leanprover-community-bot-assistant added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jul 5, 2022
@CBirkbeck
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Jul 5, 2022
Some basic definitions and results related to circle integrals of a function. These form part of #13500 



Co-authored-by: David Loeffler <d.loeffler.01@cantab.net>
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@bors
Copy link

bors bot commented Jul 5, 2022

Canceled.

@CBirkbeck
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Jul 5, 2022
Some basic definitions and results related to circle integrals of a function. These form part of #13500 



Co-authored-by: David Loeffler <d.loeffler.01@cantab.net>
@bors
Copy link

bors bot commented Jul 5, 2022

Build failed:

@CBirkbeck
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Jul 5, 2022
Some basic definitions and results related to circle integrals of a function. These form part of #13500 



Co-authored-by: David Loeffler <d.loeffler.01@cantab.net>
@bors
Copy link

bors bot commented Jul 5, 2022

Build failed (retrying...):

@CBirkbeck
Copy link
Collaborator Author

bors d+ Thanks!

I think I did things in the wrong order and now bors isnt working for me. Does it need to be delegated again?

bors bot pushed a commit that referenced this pull request Jul 6, 2022
Some basic definitions and results related to circle integrals of a function. These form part of #13500 



Co-authored-by: David Loeffler <d.loeffler.01@cantab.net>
@bors
Copy link

bors bot commented Jul 6, 2022

Build failed:

@loefflerd
Copy link
Collaborator

You need to fix the build error or it won't merge. It you click through to bors' build log it says something about a failure at circle_integral_transform.lean:62

@CBirkbeck
Copy link
Collaborator Author

bors r+

@CBirkbeck
Copy link
Collaborator Author

You need to fix the build error or it won't merge. It you click through to bors' build log it says something about a failure at circle_integral_transform.lean:62

Thanks, I couldn't see the error as I hadn't bumped my version of lean.

bors bot pushed a commit that referenced this pull request Jul 7, 2022
Some basic definitions and results related to circle integrals of a function. These form part of #13500 



Co-authored-by: David Loeffler <d.loeffler.01@cantab.net>
@bors
Copy link

bors bot commented Jul 7, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(measure_theory/integral): Circle integral transform [Merged by Bors] - feat(measure_theory/integral): Circle integral transform Jul 7, 2022
@bors bors bot closed this Jul 7, 2022
@bors bors bot deleted the circle_integral_transform branch July 7, 2022 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants