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: port Topology.ContinuousFunction.Ideals #4852

Closed
wants to merge 12 commits into from

Conversation

mo271
Copy link
Collaborator

@mo271 mo271 commented Jun 8, 2023


Open in Gitpod

mo271 added 4 commits June 8, 2023 10:25
Mathbin -> Mathlib
fix certain import statements
move "by" to end of line
add import to Mathlib.lean
@mo271 mo271 added WIP Work in progress mathlib-port This is a port of a theory file from mathlib. labels Jun 8, 2023
@mo271 mo271 added awaiting-review The author would like community review of the PR awaiting-CI and removed WIP Work in progress labels Jun 8, 2023
@mo271
Copy link
Collaborator Author

mo271 commented Jun 8, 2023

There was something wrong with the names
WeakDual.characterSpace.continuousMapEval
and
WeakDual.characterSpace.continuousMapEval_apply_apply:

It should have been an upper case "c" in CharacterSpace or one would have to change the name space
In the relevant file, @j-loreaux left the following porting note:

-- porting note: even though the capitalization of the namespace differs, it doesn't matter
-- because there is no dot notation since `characterSpace` is only a type via `CoeSort`.
namespace CharacterSpace
``

Does this mean I should use the namespace "CharacterSpace" here as well and change the name of the theorems starting with `WeakDual.characterSpace` or should I keep my change of the namespace name to "characterSpace", see porting note.
Any thoughts, @j-loreaux ?

@mo271 mo271 added help-wanted The author needs attention to resolve issues and removed awaiting-review The author would like community review of the PR awaiting-CI labels Jun 8, 2023
@j-loreaux
Copy link
Collaborator

j-loreaux commented Jun 8, 2023

@mo271 I wasn't exactly sure about this either, but my feeling is to keep the namespace uppercase (at the very least to be consistent with the previous file, or else change the namespace in that file too). It's possible we'll refactor WeakDual.characterSpace later so that it's actually a subtype, at which point the proper naming will be WeakDual.CharacterSpace anyway, making this a moot point.

Oh, and for clarity: dot notation won't work even if you make this lowercase because all Lean sees for the type is the CoeSort, not characterSpace 𝕜 A itself because that's a term, not a type.

I have addressed a lot of naming issues in this file (sorry, there were a bunch of tricky ones in there!)

@mo271
Copy link
Collaborator Author

mo271 commented Jun 8, 2023

@mo271
Copy link
Collaborator Author

mo271 commented Jun 9, 2023

Regarding the strange time-outs in the linter: Let's try nolint and adidng a porting note

@j-loreaux j-loreaux added awaiting-review The author would like community review of the PR and removed help-wanted The author needs attention to resolve issues labels Jun 13, 2023
@semorrison
Copy link
Contributor

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 Jun 13, 2023
bors bot pushed a commit that referenced this pull request Jun 14, 2023
Co-authored-by: Moritz Firsching <firsching@google.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
@bors
Copy link

bors bot commented Jun 14, 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 feat: port Topology.ContinuousFunction.Ideals [Merged by Bors] - feat: port Topology.ContinuousFunction.Ideals Jun 14, 2023
@bors bors bot closed this Jun 14, 2023
@bors bors bot deleted the port/Topology.ContinuousFunction.Ideals branch June 14, 2023 00:36
alexkeizer pushed a commit that referenced this pull request Jun 22, 2023
Co-authored-by: Moritz Firsching <firsching@google.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
semorrison pushed a commit that referenced this pull request Jun 25, 2023
Co-authored-by: Moritz Firsching <firsching@google.com>
Co-authored-by: Jireh Loreaux <loreaujy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. 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

3 participants