Remove redundant Python 2.7 code #9335

hugovk · 2020-12-22T07:07:33Z

This removes some redundant code now that Python 2.7 is no longer supported.

The bulk removes most of the use of six, except for these where I couldn't figure out the type ignores on the replacements:

from pip._vendor.six.moves import xmlrpc_client  # type: ignore
from pip._vendor.six.moves import collections_abc  # type: ignore

And also ensure_binary, ensure_text, ensure_str, reraise, perhaps these should be copied (and simplified) as utility functions?

I also didn't touch any src/pip/_vendor code.

There's definitely more Python 2 stuff that can be removed, but I didn't want this big PR to get any bigger.

uranusjr · 2020-12-22T07:52:19Z

The type: ignore thing on those two moves is due to Mypy not correctly linting them. You can replace them directly with xmlrpc.client and collection.abc.

hugovk · 2020-12-22T08:07:52Z

For example, with:

diff --git a/src/pip/_internal/resolution/resolvelib/found_candidates.py b/src/pip/_internal/resolution/resolvelib/found_candidates.py
index a669e8936..db40bf86b 100644
--- a/src/pip/_internal/resolution/resolvelib/found_candidates.py
+++ b/src/pip/_internal/resolution/resolvelib/found_candidates.py
@@ -1,7 +1,7 @@
 import itertools
 import operator
 
-from pip._vendor.six.moves import collections_abc  # type: ignore
+import collections.abc  # type: ignore
 
 from pip._internal.utils.compat import lru_cache
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
@@ -46,7 +46,7 @@ def _insert_installed(installed, others):
     return iter(candidates)
 
 
-class FoundCandidates(collections_abc.Sequence):
+class FoundCandidates(collections.abc.Sequence):
     """A lazy sequence to provide candidates to the resolver.
 
     The intended usage is to return this from `find_matches()` so the resolver

I get:

$ pre-commit run --all-files mypy
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/pip/_internal/resolution/resolvelib/found_candidates.py:49: error:
Missing
type
parameters
for
generic
type
"Sequence"
    ...ass FoundCandidates(collections.abc.Seque...
                           ^
src/pip/_internal/resolution/resolvelib/found_candidates.py:67: error:
Signature
of
"__getitem__"
incompatible
with
supertype
"Sequence"
        def __getitem__(self, index):
        ^

uranusjr · 2020-12-22T08:15:03Z

Ah OK, this one’s special because on Python 3 we need to use protocol to satisfy Mypy. But let’s not touch code for the new resolver for now, since there are more likely fixes we need to backport to 20.3 before 21.0 can drop, and adding converting code there would make backporting difficult.

pradyunsg

Thanks a lot for picking this up! <3

src/pip/_internal/operations/prepare.py

src/pip/_internal/pyproject.py

src/pip/_internal/utils/compat.py

src/pip/_internal/vcs/bazaar.py

pradyunsg

Thanks for the quick response! This is ready to go IMO, but a few inline nitpicks below. Feel free to ignore them! ^.^

src/pip/_internal/cli/progress_bars.py

tests/functional/test_download.py

tests/functional/test_install.py

jdufresne

🎉

jdufresne · 2020-12-22T15:51:17Z

src/pip/_internal/utils/compat.py

+    Returns a tuple (x, y) representing the width(x) and the height(y)
+    in characters of the terminal window.
+    """
+    return tuple(shutil.get_terminal_size())  # type: ignore


shutil.get_terminal_size() already returns a tuple. Perhaps this compat shim could be removed entirely?

IIRC this was to work around a Typeshed declaration bug. That was fixed upstream since linting jobs are passing without issues.

jdufresne · 2020-12-22T15:54:37Z

src/pip/_internal/utils/subprocess.py

+        shlex.quote(str(arg)) if isinstance(arg, HiddenText)
+        else shlex.quote(arg) for arg in args


The comment above this code is partially outdated now and perhaps the solution is too. Thoughts on?

Suggested change

shlex.quote(str(arg)) if isinstance(arg, HiddenText)

else shlex.quote(arg) for arg in args

shlex.quote(str(arg)) for arg in args

Suggestion makes sense based on the comment. What do the maintainers think (who may have more context)?

The suggestion looks good for me. In retrospect, we could have implemented __unicode__ on HiddenText and used six.text_type here (it would make the intent more obvious), but that’s all history now.

jdufresne · 2020-12-22T15:57:16Z

tests/conftest.py

@@ -166,7 +158,7 @@ def tmpdir(request, tmpdir):
        # py.path.remove() uses str paths on Python 2 and cannot
        # handle non-ASCII file names. This works around the problem by
        # passing a unicode object to rmtree().
-        shutil.rmtree(six.text_type(tmpdir), ignore_errors=True)
+        shutil.rmtree(str(tmpdir), ignore_errors=True)


The comment above this line is outdated and perhaps the solution.

So maybe just shutil.rmtree(tmpdir, ignore_errors=True)?

I think either that or simply tmpdir.remove().

tmpdir.remove(ignore_errors=True), rather.

tests/lib/__init__.py

jdufresne · 2020-12-22T16:00:20Z

tests/unit/test_wheel.py

        u"pip = pip._internal.main:pip",
        u"pip:pip = pip._internal.main:pip",
-        pytest.param(u"進入點 = 套件.模組:函式", marks=skip_if_python2),
+        u"進入點 = 套件.模組:函式",


The u-prefix could be dropped. That work could also be handled separately if preferred.

Yeah, this could be done with pyupgrade.

We'll get to that after this, I imagine.

jdufresne · 2020-12-22T16:18:27Z

Without Python 2 support, the wheel is not universal.

diff --git a/setup.cfg b/setup.cfg
index 84a959a32..10210196d 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -101,8 +101,5 @@ exclude_lines =
     # Can be set to exclude e.g. `if PY2:` on Python 3
     ${PIP_CI_COVERAGE_EXCLUDES}
 
-[bdist_wheel]
-universal = 1
-
 [metadata]
 license_file = LICENSE.txt

Edit: Done in #9339

Co-authored-by: Jon Dufresne <jon.dufresne@gmail.com>

pradyunsg · 2020-12-22T21:47:01Z

I think this PR is mergable as is, and we can iterate on this stuff as we move forward. :)

pradyunsg · 2020-12-23T17:49:40Z

🌱

Remove redundant Python 2.7 code

817ee23

Replace more six

33f48e2

hugovk force-pushed the rm-2 branch from 4217592 to 33f48e2 Compare December 22, 2020 08:22

pradyunsg reviewed Dec 22, 2020

View reviewed changes

src/pip/_internal/operations/prepare.py Outdated Show resolved Hide resolved

src/pip/_internal/pyproject.py Outdated Show resolved Hide resolved

src/pip/_internal/utils/compat.py Outdated Show resolved Hide resolved

src/pip/_internal/vcs/bazaar.py Show resolved Hide resolved

Review updates

d509a27

pradyunsg approved these changes Dec 22, 2020

View reviewed changes

src/pip/_internal/cli/progress_bars.py Show resolved Hide resolved

src/pip/_internal/cli/progress_bars.py Outdated Show resolved Hide resolved

tests/functional/test_download.py Outdated Show resolved Hide resolved

tests/functional/test_install.py Outdated Show resolved Hide resolved

hugovk added 2 commits December 22, 2020 16:38

"" is clearer than str()

2426744

Remove default allow_stderr_warning=False

209ca8d

jdufresne approved these changes Dec 22, 2020

View reviewed changes

hugovk and others added 2 commits December 22, 2020 22:41

os.curdir is already a str

9db9754

Co-authored-by: Jon Dufresne <jon.dufresne@gmail.com>

Replace compat shim with shutil.get_terminal_size()

add5cfa

hugovk force-pushed the rm-2 branch from f1f4ae8 to add5cfa Compare December 22, 2020 20:42

uranusjr mentioned this pull request Dec 23, 2020

Remove "universal" from "bdist_wheel" configuration #9339

Merged

jdufresne mentioned this pull request Dec 23, 2020

Remove obsolete "# type: ignore" comments #9340

Merged

pradyunsg merged commit 1eebb12 into pypa:master Dec 23, 2020

hugovk deleted the rm-2 branch December 23, 2020 17:50

This was referenced Dec 27, 2020

Clean up urllib imports #9371

Merged

Replace Python 2 tmpdir removal workarounds #9373

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove redundant Python 2.7 code #9335

Remove redundant Python 2.7 code #9335

hugovk commented Dec 22, 2020

uranusjr commented Dec 22, 2020

hugovk commented Dec 22, 2020

uranusjr commented Dec 22, 2020

pradyunsg left a comment

pradyunsg left a comment

jdufresne left a comment

jdufresne Dec 22, 2020

uranusjr Dec 23, 2020

jdufresne Dec 22, 2020

hugovk Dec 22, 2020

uranusjr Dec 23, 2020

jdufresne Dec 22, 2020

hugovk Dec 22, 2020

jdufresne Dec 22, 2020

uranusjr Dec 23, 2020

jdufresne Dec 22, 2020

hugovk Dec 22, 2020

pradyunsg Dec 22, 2020

jdufresne commented Dec 22, 2020 •

edited

Loading

pradyunsg commented Dec 22, 2020

pradyunsg commented Dec 23, 2020

		shlex.quote(str(arg)) if isinstance(arg, HiddenText)
		else shlex.quote(arg) for arg in args

	shlex.quote(str(arg)) if isinstance(arg, HiddenText)
	else shlex.quote(arg) for arg in args
	shlex.quote(str(arg)) for arg in args

Remove redundant Python 2.7 code #9335

Remove redundant Python 2.7 code #9335

Conversation

hugovk commented Dec 22, 2020

uranusjr commented Dec 22, 2020

hugovk commented Dec 22, 2020

uranusjr commented Dec 22, 2020

pradyunsg left a comment

Choose a reason for hiding this comment

pradyunsg left a comment

Choose a reason for hiding this comment

jdufresne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdufresne commented Dec 22, 2020 • edited Loading

pradyunsg commented Dec 22, 2020

pradyunsg commented Dec 23, 2020

jdufresne commented Dec 22, 2020 •

edited

Loading