search

jaraco / pip-run

pip-run - dynamic dependency loader for Python
MIT License
137 stars 19 forks source link

Implement PEP 723 support (script dependencies in a TOML block) #96

Closed pfmoore closed 10 months ago

pfmoore commented 10 months ago

PEP 723 support.

jaraco commented 10 months ago

Thanks Paul for the contrib. I apologize I missed it until recently (I limit my github notifications out of necessity). I'll definitely be merging this or something similar soon. Thanks @bswck for the additional review. I'll take the suggestions under consideration.

pfmoore commented 10 months ago

Thanks @jaraco - if you want me to make any changes, just let me know.

@bswck Sorry if it seemed I was suggesting your review wasn't welcome - on the contrary, I appreciate the feedback. My only concern was that I didn't want to make too many changes (increasing churn on the PR) without getting an indication from the project maintainers on their opinion. I'm mostly indifferent to matters of code structure (unless they actively harm readability/maintainability IMO) and so I find it all to easy to get sucked into making too many changes that are of marginal benefit if I don't push back a little.

pfmoore commented 10 months ago

One specific point - the coverage check is failing, which I assume is related to the issue @bswck pointed out, and so I need to add # pragma: no cover somewhere. I'm not 100% sure where, though - I assume what gets covered will depend on the Python version, so do I need to add it to all of the try...except block?

Also, if anyone can save me getting it wrong, what's the right form for combining a type: ignore and pragma: no cover comments on the same line?

jaraco commented 10 months ago

Hmm. While working on a test for the "multiple blocks" error, I've encountered an unexpected behavior with the regex. Consider this script:

# /// script
# ///

In my mind, that should be legal, a degenerate (empty) block, but the regex expects at least one line between the starting and ending line. Since the text of the PEP is silent on the matter, it seems it should be valid form (in the same way that the empty string is valid Python and valid TOML).

pfmoore commented 10 months ago

Does this format supersede the "comment" format or should all three formats be supported indefinitely? If the former, let's log an issue to mark the comment form as deprecated.

IMO, this should supersede the comment format (PEP 722, which standardised the comment format, was rejected in favour of PEP 723). I've raised https://github.com/jaraco/pip-run/issues/97 for this.