-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC update documentation on re-building Cython extensions #26334
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
DOC update documentation on re-building Cython extensions #26334
Conversation
Thanks for figuring this out. While reading the diff I realised that I have no idea what this PEP517 is about, and why I might want to (not) use it. Is there a short explanation of what the PEP does? (I've read the abstract of the PEP but left none the wiser) Similarly, why do we need this new flag to Do we need to add the The pedant in me would prefer if we spelled out all the command line flags (so And finally, some times we explain what |
I have no idea what pep517 does, and I don't really care (it's given me headaches). But w/o it, the The I don't mind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is an immediate and welcome improvement to an unfortunately common headache. I would also like to see in separate PRs:
- A brief explanation of PEP 517 -- perhaps as a note or an aside.
- Making the commands uniform (e.g. sometimes we use
-e
and sometimes--editable
, as well as different orders of flags).
Personally I think using the shortened -e
is preferable since readers can easily look up the specific flag if they are curious about it, and it meaningfully shortens these already-long commands.
Reference Issues/PRs
towards #25985
What does this implement/fix? Explain your changes.
Updates the documentation on re-building Cython extensions to provide better guidance for new contributors. I also updated the
pip install -v --no-build-isolation -e .
command topip install -v --no-use-pep517 --no-build-isolation -e .
in the documentation to prevent potential errors during the installation process.Any other comments?