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

Update Python < 3.8 (3.2 to 3.7) obsolete code and comments #4096

Merged
merged 12 commits into from
Jan 24, 2024

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Oct 27, 2023

Summary of changes

Prompted by https://github.com/pypa/setuptools/pull/3979/files#r1257406447
Removing obsolete code and workarounds benefit type-checkers.

This PR avoids touching any of the _vendor, _distutils, extern or deprecated folders

More details for specific changes are added as comments below.

Pull Request Checklist

import imp as _imp

try:
FileExistsError
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FileExistsError added in Python 3.3

pkg_resources/__init__.py Outdated Show resolved Hide resolved
@@ -257,8 +257,9 @@ def finalize_options(self): # noqa: C901 # is too complex (25) # FIXME
'py_version_nodot': f'{sys.version_info.major}{sys.version_info.minor}',
'sys_prefix': self.config_vars['prefix'],
'sys_exec_prefix': self.config_vars['exec_prefix'],
# Only python 3.2+ has abiflags
# Only POSIX systems have abiflags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'abiflags': getattr(sys, 'abiflags', ''),
# Only python 3.9+ has platlibdir
Copy link
Contributor Author

@Avasam Avasam Oct 27, 2023

Choose a reason for hiding this comment

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

@@ -138,8 +138,7 @@ def patch_for_msvc_specialized_compiler():
Patch functions in distutils to use standalone Microsoft Visual C++
compilers.
"""
# import late to avoid circular imports on Python < 3.5
msvc = import_module('setuptools.msvc')
from . import msvc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically a revert of b7b9cb2
Unless this still happens in newer versions, in which case the comment should be updated instead.

@Avasam Avasam force-pushed the update-python-3.2-to-3.6-code branch from 714420f to 01ca1f2 Compare October 27, 2023 20:55
@Avasam Avasam mentioned this pull request Oct 27, 2023
2 tasks
@Avasam Avasam changed the title Update Python < 3.7 (3.2 to 3.6) obsolete code and comments Update Python < 3.8 (3.2 to 3.7) obsolete code and comments Nov 7, 2023
@Avasam Avasam force-pushed the update-python-3.2-to-3.6-code branch from bb4ff2f to 05ca49d Compare November 7, 2023 03:39
@Avasam Avasam force-pushed the update-python-3.2-to-3.6-code branch from 05ca49d to d97640a Compare November 7, 2023 03:42
index = setuptools.package_index.PackageIndex(hosts=('www.example.com',))

# issue #160
if sys.version_info[0] == 2 and sys.version_info[1] == 7:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Python 2.7 test?

Copy link
Contributor

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution. I see that you already started discussing with Jason about some of these changes, so I will leave it to him, but I just wanted to point out that some modifications on the docs might need some review. I left some comments (that also might also apply to the following parts of the documents, not only the preceeding lines in the Github diff).

docs/userguide/declarative_config.rst Show resolved Hide resolved
docs/userguide/pyproject_config.rst Show resolved Hide resolved
docs/userguide/quickstart.rst Show resolved Hide resolved
@Avasam Avasam force-pushed the update-python-3.2-to-3.6-code branch from 2ffc63c to c242f74 Compare November 8, 2023 00:03
docs/userguide/quickstart.rst Outdated Show resolved Hide resolved
@Avasam
Copy link
Contributor Author

Avasam commented Nov 22, 2023

pkg_resources/__init__.py (66.7%): Missing lines 23
pkg_resources/tests/test_pkg_resources.py (100%)
setuptools/command/bdist_egg.py (100%)
setuptools/command/build.py (100%)
setuptools/command/dist_info.py (100%)
setuptools/monkey.py (100%)
setuptools/tests/test_distutils_adoption.py (100%)
setuptools/tests/test_find_packages.py (100%)
setuptools/tests/test_find_py_modules.py (100%)
-------------
Total:   12 lines
Missing: 1 line
Coverage: 91%
-------------

diffcov: commands[2]> diff-cover coverage.xml --compare-branch=origin/main --fail-under=100
Failure. Coverage is below 100%.

I'm guessing coverage of pkg_resources/__init__.py was already below 100%, because if anything my changes should make coverage go up as there's less branches.

@Avasam Avasam mentioned this pull request Nov 28, 2023
2 tasks
which is a part of the standard library since Python 3.8. For older versions of
Python, its backport :pypi:`importlib_metadata` should be used. While using the
backport, the only change that has to be made is to replace ``importlib.metadata``
which is a part of the standard library since Python 3.8 and is non-provisional
Copy link
Contributor Author

@Avasam Avasam Nov 28, 2023

Choose a reason for hiding this comment

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

I think the issue/reason to use importlib_metadata for Python older than 3.10 is python/importlib_metadata#396 ? Is that something we want to mention?

Copy link
Member

Choose a reason for hiding this comment

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

There are other reasons too - basically for early adoption of any behavior that was added to importlib_metadata but isn't yet in the supported Pythons. Still, that seems like something for cpython/importlib_metadata to advertise.

@Avasam
Copy link
Contributor Author

Avasam commented Jan 12, 2024

@abravalheri There's been some small updates since your last review, so I reset your vote. There's also a ton of overlap with #4138 from @jaraco

@jaraco
Copy link
Member

jaraco commented Jan 24, 2024

Thanks for all the work here. Let's get this merged. We can tweak more later if needed.

@jaraco jaraco merged commit 1c4946a into pypa:main Jan 24, 2024
21 of 23 checks passed
@Avasam Avasam deleted the update-python-3.2-to-3.6-code branch January 24, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants