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

Convert to an installable python package #148

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Dec 17, 2021

This isn't a full conversion, but should still work after this restructure.

The motivation here is to try and separate out "things specific to mathlib" from "things other lean projects might want". It also means we don't need to worry about any python file names clashing with builtin modules, as they're now namespaced to lean_doc_gen.

Future ideas intended for later PRs:

  • Extract the command line parsing from __init__.py into its own file
  • Convert some shell scripts to python scripts
  • Extract mathlib config to a config file in the mathlib repo.

Since bors isn't enabled in this repository, if we do decide to merge this we should either squash merge through github, or I should manually do so locally.

@eric-wieser eric-wieser marked this pull request as ready for review December 17, 2021 01:09
@@ -36,4 +36,4 @@ bash "$source"scripts/mk_all.sh
lean src/entrypoint.lean >export.json
bash "$source"scripts/rm_all.sh

python3 print_docs.py -r "$source" -w "$site_root" "${args[@]}"
lean-doc-gen -r "$source" -w "$site_root" "${args[@]}"
Copy link
Member

Choose a reason for hiding this comment

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

Please make it possible to run doc-gen and gen_docs without installing the package. python -m lean_doc_gen fails here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do we care about supporting non-installed packages?

Note that even with the package installed, python -m lean_doc_gen fails here; but that's only because it picks up the non-installed version in the current directory. If I moved lean_doc_gen to python_src/lean_doc_gen, or just changed the working directory when running this script, then that wouldn't happen.

Copy link
Member

Choose a reason for hiding this comment

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

Why do we care about supporting non-installed packages?

Because I'm using that for development and I'm not keen on changing my ways. If we require installation then the readme must be updated. I believe there was a flag where pip just symlinks the package during installation, so that you don't need to reinstall after every change.

@gebner
Copy link
Member

gebner commented Dec 20, 2021

What is the story for the *.lean files here? Are these installed as well?

Since bors isn't enabled in this repository, if we do decide to merge this we should either squash merge through github, or I should manually do so locally.

Yeah sure, I can squash.

@eric-wieser
Copy link
Member Author

What is the story for the *.lean files here? Are these installed as well?

No, not yet - but the python code doesn't use these at all yet.

@gebner
Copy link
Member

gebner commented Dec 20, 2021

No, not yet - but the python code doesn't use these at all yet.

Hmm, but is there anything you can do with the python code alone? Related question: are the CSS/JS files copied as well? I can't find any relevant changes in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants