-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
Removed the unnecessary setuptools package dependency for Python package #7511
Removed the unnecessary setuptools package dependency for Python package #7511
Conversation
@@ -241,7 +241,7 @@ def get_option_from_sys_argv(option_str): | |||
os.environ['PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION'] = 'cpp' | |||
|
|||
# Keep this list of dependencies in sync with tox.ini. | |||
install_requires = ['six>=1.9', 'setuptools'] | |||
install_requires = ['six>=1.9'] |
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.
Per the comment, can you verify whether any change to tox.ini
is required?
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.
Funnily enough - tox.ini was NOT in sync with this file, in the one way that it was missing setuptools :).
@haberman, and update on this? |
@haberman , Friendly ping? |
Friendly ping again? |
…age. The setuptools package was added to allow definition of namespaces using the now outdated (and discouraged from use) pkg_resources-style. The code here, for a long while now, uses a try/except (`ImportError`) protection around the setuptools code, and falls back to the more encoraged pkgutil-style. Removing this library won't affect any current workflow, and in the case of setuptools not found, it'll actually use a more modern (and encouraged) flow.
e2a642c
to
d13288e
Compare
Sorry for letting this slip. This looks fine to me, but I'd feel a little better if someone from our team who knows Python packaging stuff better than me took a look. @dlj-NaN could you take a look? |
@dlj-NaN, friendly ping? |
The setuptools package was added to allow definition of namespaces using
the now outdated (and discouraged from use) pkg_resources-style.
The code here, for a long while now, uses a try/except (
ImportError
)protection around the setuptools code, and falls back to the more
encoraged pkgutil-style.
Removing this library won't affect any current workflow, and in the case
of setuptools not found, it'll actually use a more modern (and
encouraged) flow.
More info: https://packaging.python.org/guides/packaging-namespace-packages/