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

Remove import re from script template #13166

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

hukkin
Copy link
Contributor

@hukkin hukkin commented Jan 14, 2025

Overrides distlib.scripts.ScripMaker.script_template with a template that doesn't import re.

Closes #13165

@notatallshaw
Copy link
Member

notatallshaw commented Jan 14, 2025

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.

@pfmoore
Copy link
Member

pfmoore commented Jan 14, 2025

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 -*-
Copy link
Member

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

Speed up small CLI tools by removing import re from the excecutable template
3 participants