Remove import re from script template
#13166
Conversation
|
While I would personally prefer the distlib script to contain the minimal number of imports, I am against diverging from distlibs default in pip. If distlib maintainers don't want to take on the burden of this putting that burden on to pip has all the same problems and more, pip is not running distlib's test suite, does not have distlib's expertise, and could fall out of sync with changes in distlib. And this would affect a huge number of users. I'll say -0, because it is actually fairly simple, and maybe some other maintainer is enthusiastic to support this. |
|
Added some code review comments, but see my comment on #13165 for my broader views. |
| @@ -412,6 +412,17 @@ def _raise_for_invalid_entrypoint(specification: str) -> None: | |||
|
|
|||
|
|
|||
| class PipScriptMaker(ScriptMaker): | |||
| # Override distlib's default script template with one that | |||
| # doesn't import `re` module, allowing scripts to load faster. | |||
| script_template = r"""# -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
Given pip doesn't work with Python 2 anymore # -*- coding: utf-8 -*- can be removed right?
https://peps.python.org/pep-3120/ / https://github.com/asottile/pyupgrade?tab=readme-ov-file#-coding--comment / https://docs.astral.sh/ruff/rules/utf8-encoding-declaration/
There was a problem hiding this comment.
I think so.
I also wonder if it’d be worthwhile to conditionally use removesuffix.
There was a problem hiding this comment.
Given pip doesn't work with Python 2 anymore # -- coding: utf-8 -- can be removed right?
I think so too, I can remove it. My initial thought was to omit any fancy improvements from this PR, and simply replace the re.sub call with something that mimics it closely, so if distlib updates the template for some reason, it may be easier to track and copy the changes here.
I also wonder if it’d be worthwhile to conditionally use removesuffix.
It could be used, and could be used unconditionally, now that there is only one suffix to remove in this edit. Pip still supports Python 3.8 and removesuffix was added in 3.9, so we're not there yet.
One micro-optimization that can also be done is replace sys.exit with raise SystemExit to avoid one function call, but that may be a bit too much :D
There was a problem hiding this comment.
One micro-optimization that can also be done is replace sys.exit with raise SystemExit to avoid one function call, but that may be a bit too much :D
It appears to be to a C function though, so it may not be a clear win, especially given sys needs to be imported anyway.
Overrides
distlib.scripts.ScripMaker.script_templatewith a template that doesn'timport re.Closes #13165