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(Complex/UpperHalfPlane): add vertical strips #12443

Closed
wants to merge 8 commits into from

Conversation

CBirkbeck
Copy link
Collaborator

@CBirkbeck CBirkbeck commented Apr 26, 2024

We define the vertical strips that are needed for proving Eisenstein series are modular forms #10377 . We also add the definition of sqrt{-1} as an elements of the upper half plane. Note that this is no longer needed for the modular forms PRs but its good to have.

Sorry about the typo in the PR name, its not my day!


Open in Gitpod

…s and make i as an element of the upper half plane
@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@CBirkbeck CBirkbeck added awaiting-review The author would like community review of the PR awaiting-CI and removed new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! labels Apr 26, 2024
@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@loefflerd
Copy link
Collaborator

Since the vertical-strip stuff doesn't use the metric any more, it probably shouldn't be in UpperHalfPlane/Metric.lean.

@loefflerd loefflerd 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 Apr 26, 2024
@CBirkbeck
Copy link
Collaborator Author

Since the vertical-strip stuff doesn't use the metric any more, it probably shouldn't be in UpperHalfPlane/Metric.lean.

Good point, I'm not quite sure where to place it, I guess it could go into topology?

@CBirkbeck CBirkbeck added awaiting-review The author would like community review of the PR and removed new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! awaiting-author A reviewer has asked the author a question or requested changes labels Apr 26, 2024
@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
Copy link
Collaborator

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Looks straightforward; I just have three minor comments. Thanks for your contribution!

Mathlib/Analysis/Complex/UpperHalfPlane/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Analysis/Complex/UpperHalfPlane/Topology.lean Outdated Show resolved Hide resolved
Mathlib/Analysis/Complex/UpperHalfPlane/Topology.lean Outdated Show resolved Hide resolved
@CBirkbeck CBirkbeck removed the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@CBirkbeck CBirkbeck removed the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@github-actions github-actions bot added the new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! label Apr 26, 2024
@loefflerd
Copy link
Collaborator

I think for consistency with mathlib's naming conventions, strip_mem_iff should probably be mem_verticalStrip_iff, and similarly subset_verticalStrip_of_isCompact for the other one. Otherwise looks good, I'd be happy to recommend this for merging modulo that change.

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.

LGTM, thanks!

maintainer merge

Copy link

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

@riccardobrasca
Copy link
Member

Thanks!

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 Apr 30, 2024
mathlib-bors bot pushed a commit that referenced this pull request Apr 30, 2024
We define the vertical strips that are needed for proving Eisenstein series are modular forms #10377 . We also add the definition of sqrt{-1} as an elements of the upper half plane. Note that this is no longer needed for the modular forms PRs but its good to have.

Sorry about the typo in the PR name, its not my day!



Co-authored-by: Chris Birkbeck <c.birkbeck@uea.ac.uk>
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 30, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(Complex/UpperHalfPlane): add vertical strips [Merged by Bors] - feat(Complex/UpperHalfPlane): add vertical strips Apr 30, 2024
@mathlib-bors mathlib-bors bot closed this Apr 30, 2024
@mathlib-bors mathlib-bors bot deleted the upper_half_plane_stip_lemmas branch April 30, 2024 15:37
apnelson1 pushed a commit that referenced this pull request May 12, 2024
We define the vertical strips that are needed for proving Eisenstein series are modular forms #10377 . We also add the definition of sqrt{-1} as an elements of the upper half plane. Note that this is no longer needed for the modular forms PRs but its good to have.

Sorry about the typo in the PR name, its not my day!



Co-authored-by: Chris Birkbeck <c.birkbeck@uea.ac.uk>
callesonne pushed a commit that referenced this pull request May 16, 2024
We define the vertical strips that are needed for proving Eisenstein series are modular forms #10377 . We also add the definition of sqrt{-1} as an elements of the upper half plane. Note that this is no longer needed for the modular forms PRs but its good to have.

Sorry about the typo in the PR name, its not my day!



Co-authored-by: Chris Birkbeck <c.birkbeck@uea.ac.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer-merge new-contributor This PR was made by a contributor with fewer than 5 merged PRs. Welcome to the community! ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants