Skip to content

Migrating alpha file import to lean + adding testing - #1552

Merged
jstoobysmith merged 11 commits into
leanprover-community:masterfrom
FergusMunro:migratingAlphaFileImportToLean
Aug 21, 2026
Merged

Migrating alpha file import to lean + adding testing#1552
jstoobysmith merged 11 commits into
leanprover-community:masterfrom
FergusMunro:migratingAlphaFileImportToLean

Conversation

@FergusMunro

Copy link
Copy Markdown
Contributor

Creates alphaFileImports.lean, replacing alphaFileImports.py. Adjusts workflows and AGENTS.md to use this new script.

Adds alphaFileImports.lean and noAlphaImports.lean to lint_all.lean.

Adds ./Meta/test directory containing a dummy project test_project and a test script testImportScripts.lean that tests alphaFileImports.lean and noAlphaImports.lean fail correctly on the dummy project.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for this pull-request (PR). If this is your first PR, welcome to the community!

Below is what will happen next. Please read carefully if you are not familiar with the process. You may open other PRs while this one is being reviewed, and can stack PRs on top of each other, so don't let these steps slow you down.

  1. Some automated checks will be run on your PR. You can see the results of these checks at the buttom of your PR page. If any of these checks fail, you will need to fix the issues before your PR can be merged. You can learn more about these here, including how to run them locally, which is sometimes quicker than relying on the GitHub Actions. If you have never had a PR merged before, you may have to wait for a reviewer to manually start these checks (this is for security).

  2. A reviewer will look at your PR and may ask you to make changes. This may happen a couple of days after you submit your PR, so you may need to be patient. But it should not be longer than that - if it is please bring it to the attention of the community on the Zulip. The level of review will depend on where your PR is submitted. If it is submitted to ./Physlib or ./QuantumInfo, the review will be more thorough than if it is submitted to ./PhyslibAlpha. You can find out more about what the review process is looking for in our review guidelines. If a reviewer adds an awaiting-author label to your PR, address the review comments, then please remove that label by adding a comment with -awaiting-author. This helps us keep track of reviews.

  3. The reviewer will either approve your PR, or request more changes (in which case we return to step 2). Once your PR is approved, it will be merged by a maintainer, this should happen shortly after approval, though you may get more comments at this stage.

Tip: The easiest way to get have a fast review is to submit a PR that is small and self-contained, and has clear documentation explaining why things are the way they are in your chages.

If you have any problems or questions, please reach out to the community on the Zulip.

@github-actions github-actions Bot added the CI label Aug 19, 2026
@jstoobysmith jstoobysmith self-assigned this Aug 19, 2026
Comment thread scripts/lint_all.lean
Comment thread Meta/test/test_project/TestProject.lean Outdated

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.

This is not quite what I had in mind for the Meta file.
I was thinking e.g. withing the ./PhyslibAlpha project not as a seperate project. Maybe we should save these tests for a future PR, and look at Mathlib to see what they do here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wasn't too happy with it either. I've temporarily removed it and when I get the chance I'll try to look at Mathlib.

@jstoobysmith

Copy link
Copy Markdown
Member

awaiting-author

@github-actions github-actions Bot added the awaiting-author A reviewer has asked the author a question or requested changes label Aug 20, 2026
@github-actions github-actions Bot added small and removed medium labels Aug 20, 2026

def checkAllFilesImported (directory : String) (mainFilePath : String) : (IO Bool) := do
let modules : HashSet String := HashSet.ofArray $ (← getFilePaths directory).map extractModuleNameFromFilePath

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.

Maybe would be nice to remove these new lines from within definitions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've removed the newlines, if there's some other style changes you'd like to make let me know.

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.

For meta programming I think it is a bit more loose than the Lean code itself. But the main ones are this, definitions should be written like checkAllFilesImported not like check_all_files_imported, and generally make things concise and readable where possible.

Comment thread scripts/PhyslibAlpha/alphaFileImports.lean Outdated

@jstoobysmith jstoobysmith left a comment

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.

Approved - looks good. Thanks

@jstoobysmith jstoobysmith added ready-to-merge This PR is approved and will be merged shortly and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 20, 2026
@jstoobysmith
jstoobysmith merged commit 665cf2b into leanprover-community:master Aug 21, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI ready-to-merge This PR is approved and will be merged shortly small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants