-
-
Notifications
You must be signed in to change notification settings - Fork 611
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
Support scripts with inline script metadata as input files #2107
base: main
Are you sure you want to change the base?
Conversation
2f914a1
to
71d54de
Compare
@webknjaz please review; there isn't any other active maintainer left, as it seems.. |
What's the use-case for this? |
Hmm, suppose you have a Python script that has dependencies, is probably often in use and not part of a package with corresponding metadata; then setting up an environment fastly would be possible through this functionality. |
@WhyNotHugo would you like to review this? We're really short on help ATM. |
dedent( | ||
""" | ||
# /// script | ||
# dependencies = [ |
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.
Should we also have tests for extras?
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.
Oh, there's no explicit extras in the spec. But how about requires-python
?
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'm not sure how that would influence the build process?
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.
It should reject incompatible runtimes, for example.
@@ -43,6 +49,10 @@ | |||
DEFAULT_REQUIREMENTS_OUTPUT_FILE = "requirements.txt" | |||
METADATA_FILENAMES = frozenset({"setup.py", "setup.cfg", "pyproject.toml"}) | |||
|
|||
INLINE_SCRIPT_METADATA_REGEX = ( |
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 recommend using (?x)
to be able to make this multiline with comments.
# piping from stdin, we need to briefly save the input from stdin | ||
# to a temporary file and have pip read that. also used for | ||
if src_file == "-" or ( | ||
os.path.basename(src_file).endswith(".py") and not is_setup_file |
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.
pathlib is usually nicer
os.path.basename(src_file).endswith(".py") and not is_setup_file | |
Path(src_file).suffix == ".py" and not is_setup_file |
# reading requirements from install_requires in setup.py. | ||
if os.path.basename(src_file).endswith(".py"): |
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.
if os.path.basename(src_file).endswith(".py"): | |
if Path(src_file).suffix == ".py": |
@chrysle sorry, I didn't have time lately. I'll try to come back for another review round during/after EuroPython. |
71d54de
to
792e034
Compare
@chrysle I clicked “rebase” and it looks like there's some actionable feedback in the earlier comments. |
Closes #2027.
Contributor checklist
Maintainer checklist
backwards incompatible
,feature
,enhancement
,deprecation
,bug
,dependency
,docs
orskip-changelog
as they determine changelog listing.