Update mypy to v0.770 #8235
Update mypy to v0.770 #8235
Conversation
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
9d70280 to
d82eabe
Compare
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
d82eabe to
31fba02
Compare
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
31fba02 to
4ff0f56
Compare
|
For some reason, the tests which interact with external svn and hg repositories and running in the azure pipeline are timing out, causing the checks to fail. I think this is unrelated to the changes made here. |
|
As per python/mypy#8816 (comment) , seems like v0.770 fixed this, so we have a few options to move this PR ahead.
Let me know what we want to do here, and I will add another commit in this PR accordingly to make either of the 3 options happen. |
|
Ah well. Let's bump to 0.770 and see what happens. :) |
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
4ff0f56 to
feaa76a
Compare
Update mypy to 0.770 in latest commit and readded Edit: This raised a new error while running Edit 2: 03ee641 fixed the issue below. |
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
eadc853 to
03ee641
Compare
|
There has been changes to the PR in order to support mypy v0.770, so the previous approval no longer holds true. Hence I am going ahead and re-requesting an review. |
I think this is a call-site problem; we should add a |
pradyunsg
added
skip news
As per your suggestion, I tried I also observed that as per the following function pip/src/pip/_internal/operations/install/wheel.py Lines 71 to 79 in a879bc4 we open the file in string mode for Python 3, and mypy complains when we use I think that casting record_file to Please let me know if there is a gap in my understanding here. |
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
2 times, most recently
from
9a12a49 to
bd07bfb
Compare
|
|
Could you tell me what should I be considering conceptually when trying to write the universal cast? I actually added that I was under the impression that this is because of pip/src/pip/_internal/operations/install/wheel.py Lines 71 to 79 in a879bc4 but I think that's the wrong assumption IIUC, mypy only complains in PY3 and not PY2 because of pip/src/pip/_internal/utils/filesystem.py Line 24 in a879bc4 because when I change it to |
|
On thinking about this further, I think this might be a bug in typeshed, I have filed python/typeshed#3997 for it. If that is a valid bug, I can go ahead and add |
|
What's the |
|
I went ahead and committed the second option from the above comment, adding The first options causes tests to fail with |
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
from
2e2c3ad to
09b52f2
Compare
|
Huh, surprising that This seems to work for me (based on your branch): diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py
index 142a2f5e..ab2f5779 100644
--- a/src/pip/_internal/operations/install/wheel.py
+++ b/src/pip/_internal/operations/install/wheel.py
@@ -38,9 +38,8 @@ from pip._internal.utils.wheel import parse_wheel
if MYPY_CHECK_RUNNING:
from email.message import Message
- import typing # noqa F401
from typing import (
- Dict, List, Optional, Sequence, Tuple, Any,
+ Dict, IO, List, Optional, Sequence, Tuple, Any,
Iterable, Iterator, Callable, Set,
)
@@ -606,7 +605,7 @@ def install_unpacked_wheel(
# Python 3 (typing.IO[Any]) and Python 2 (typing.BinaryIO),
# leading us to explicitly cast to typing.IO[Any] as a workaround
# for bad Python 2 behaviour
- record_file_obj = cast('typing.IO[Any]', record_file)
+ record_file_obj = cast('IO[Any]', record_file)
writer = csv.writer(record_file_obj)
writer.writerows(sorted_outrows(rows)) # sort to simplify testing |
I actually did something similar to the patch you provided, but then Hence I went with |
|
What version of pyflakes and flake8 are you using? This can happen with older versions that don’t understand the type comments. See PyCQA/pyflakes#447. |
The I now see that we are using flake8 Lines 19 to 20 in f34f8d5 and the fix for the issue tagged above PyCQA/pyflakes#479 got in Feb 2020. I also tried the the latest version of flake8, $ git diff src/pip/_internal/operations/install/wheel.py
diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py
index 142a2f5e..ab2f5779 100644
--- a/src/pip/_internal/operations/install/wheel.py
+++ b/src/pip/_internal/operations/install/wheel.py
@@ -38,9 +38,8 @@ from pip._internal.utils.wheel import parse_wheel
if MYPY_CHECK_RUNNING:
from email.message import Message
- import typing # noqa F401
from typing import (
- Dict, List, Optional, Sequence, Tuple, Any,
+ Dict, IO, List, Optional, Sequence, Tuple, Any,
Iterable, Iterator, Callable, Set,
)
@@ -606,7 +605,7 @@ def install_unpacked_wheel(
# Python 3 (typing.IO[Any]) and Python 2 (typing.BinaryIO),
# leading us to explicitly cast to typing.IO[Any] as a workaround
# for bad Python 2 behaviour
- record_file_obj = cast('typing.IO[Any]', record_file)
+ record_file_obj = cast('IO[Any]', record_file)
writer = csv.writer(record_file_obj)
writer.writerows(sorted_outrows(rows)) # sort to simplify testing
Same thing with v2.2.0 of |
|
Okay, so if I use $ git diff src/pip/_internal/operations/install/wheel.py
diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py
index 142a2f5e..120c48a4 100644
--- a/src/pip/_internal/operations/install/wheel.py
+++ b/src/pip/_internal/operations/install/wheel.py
@@ -32,16 +32,15 @@ from pip._internal.models.direct_url import DIRECT_URL_METADATA_NAME, DirectUrl
from pip._internal.utils.filesystem import adjacent_tmp_file, replace
from pip._internal.utils.misc import captured_stdout, ensure_dir, hash_file
from pip._internal.utils.temp_dir import TempDirectory
-from pip._internal.utils.typing import MYPY_CHECK_RUNNING, cast
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.unpacking import current_umask, unpack_file
from pip._internal.utils.wheel import parse_wheel
if MYPY_CHECK_RUNNING:
from email.message import Message
- import typing # noqa F401
from typing import (
- Dict, List, Optional, Sequence, Tuple, Any,
- Iterable, Iterator, Callable, Set,
+ Dict, IO, List, Optional, Sequence, Tuple, Any,
+ Iterable, Iterator, Callable, Set, cast
)
from pip._internal.models.scheme import Scheme
@@ -606,7 +605,7 @@ def install_unpacked_wheel(
# Python 3 (typing.IO[Any]) and Python 2 (typing.BinaryIO),
# leading us to explicitly cast to typing.IO[Any] as a workaround
# for bad Python 2 behaviour
- record_file_obj = cast('typing.IO[Any]', record_file)
+ record_file_obj = cast('IO[Any]', record_file)
writer = csv.writer(record_file_obj)
writer.writerows(sorted_outrows(rows)) # sort to simplify testingBut then the functional tests associated to For e.g. Failing test_install_wheel.py::test_install_from_future_wheel_version |
This needs us to import from typing at runtime, which we can't do (thanks Python 2!). :( Hey @asottile! o/ Any suggestions on how to avoid this situation we're in, where we're trying to get our |
Yea... MYPY_CHECK_RUNNING is always False at runtime, but mypy does consider the body of that block during type analysis (by design). The main "trick" in our don't-import-typing-at-runtime approach, is that all |
I just tried it locally, seems to be working already? |
May I know what you tried locally? Did you check out the PR locally and ran |
oh, I was just running |
So we have pip._internal.utils.typing where we use The The following minimal example should be able to help you understand the problem better. import io
# pip/src/pip/_internal/utils/typing.py repro
MYPY_CHECK_RUNNING = False
if MYPY_CHECK_RUNNING:
from typing import cast, IO, Any
else:
# typing's cast() is needed at runtime, but we don't want to import typing.
# Thus, we use a dummy no-op version, which we tell mypy to ignore.
def cast(_, value): # type: ignore
return value
# pip/src/pip/_internal/operations/install/wheel.py repro
with io.BytesIO() as record_file:
record_file_obj = cast('IO[Any]', record_file)Running |
|
this patch passes without noqa: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 3e2c0a2a..0a7847c1 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -17,7 +17,7 @@ repos:
exclude: .patch
- repo: https://gitlab.com/pycqa/flake8
- rev: 3.7.9
+ rev: 3.8.1
hooks:
- id: flake8
exclude: tests/data
diff --git a/src/pip/_internal/commands/show.py b/src/pip/_internal/commands/show.py
index a61294ba..3e09e41f 100644
--- a/src/pip/_internal/commands/show.py
+++ b/src/pip/_internal/commands/show.py
@@ -93,7 +93,7 @@ def search_packages_info(query):
# RECORDs should be part of .dist-info metadatas
if dist.has_metadata('RECORD'):
lines = dist.get_metadata_lines('RECORD')
- paths = [l.split(',')[0] for l in lines]
+ paths = [line.split(',')[0] for line in lines]
paths = [os.path.join(dist.location, p) for p in paths]
file_list = [os.path.relpath(p, dist.location) for p in paths]
diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py
index 142a2f5e..a548553f 100644
--- a/src/pip/_internal/operations/install/wheel.py
+++ b/src/pip/_internal/operations/install/wheel.py
@@ -32,15 +32,16 @@ from pip._internal.models.direct_url import DIRECT_URL_METADATA_NAME, DirectUrl
from pip._internal.utils.filesystem import adjacent_tmp_file, replace
from pip._internal.utils.misc import captured_stdout, ensure_dir, hash_file
from pip._internal.utils.temp_dir import TempDirectory
-from pip._internal.utils.typing import MYPY_CHECK_RUNNING, cast
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
from pip._internal.utils.unpacking import current_umask, unpack_file
from pip._internal.utils.wheel import parse_wheel
-if MYPY_CHECK_RUNNING:
+if not MYPY_CHECK_RUNNING:
+ from pip._internal.utils.typing import cast
+else:
from email.message import Message
- import typing # noqa F401
from typing import (
- Dict, List, Optional, Sequence, Tuple, Any,
+ cast, Dict, IO, List, Optional, Sequence, Tuple, Any,
Iterable, Iterator, Callable, Set,
)
@@ -606,7 +607,7 @@ def install_unpacked_wheel(
# Python 3 (typing.IO[Any]) and Python 2 (typing.BinaryIO),
# leading us to explicitly cast to typing.IO[Any] as a workaround
# for bad Python 2 behaviour
- record_file_obj = cast('typing.IO[Any]', record_file)
+ record_file_obj = cast('IO[Any]', record_file)
writer = csv.writer(record_file_obj)
writer.writerows(sorted_outrows(rows)) # sort to simplify testing
diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py
index 09031825..65cd3888 100644
--- a/src/pip/_internal/utils/misc.py
+++ b/src/pip/_internal/utils/misc.py
@@ -541,7 +541,7 @@ class FakeFile(object):
"""Wrap a list of lines in an object with readline() to make
ConfigParser happy."""
def __init__(self, lines):
- self._gen = (l for l in lines)
+ self._gen = iter(lines)
def readline(self):
try:
diff --git a/src/pip/_internal/wheel_builder.py b/src/pip/_internal/wheel_builder.py
index fcaeeb6c..261380df 100644
--- a/src/pip/_internal/wheel_builder.py
+++ b/src/pip/_internal/wheel_builder.py
@@ -36,7 +36,9 @@ logger = logging.getLogger(__name__)
def _contains_egg_info(
- s, _egg_info_re=re.compile(r'([a-z0-9_.]+)-([a-z0-9_.!+-]+)', re.I)):
+ s,
+ _egg_info_re=re.compile(
+ r'([a-z0-9_.]+)-([a-z0-9_.!+-]+)', re.IGNORECASE)):
# type: (str, Pattern[str]) -> bool
"""Determine whether the string looks like an egg_info.
diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py
index 0c00060a..40b0c3c7 100644
--- a/tests/functional/test_install_reqs.py
+++ b/tests/functional/test_install_reqs.py
@@ -420,7 +420,7 @@ def test_install_with_extras_from_install(script, data):
)
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]')
- assert script.site_packages / 'singlemodule.py'in result.files_created
+ assert script.site_packages / 'singlemodule.py' in result.files_created
def test_install_with_extras_joined(script, data):
@@ -432,7 +432,7 @@ def test_install_with_extras_joined(script, data):
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]'
)
assert script.site_packages / 'simple' in result.files_created
- assert script.site_packages / 'singlemodule.py'in result.files_created
+ assert script.site_packages / 'singlemodule.py' in result.files_created
def test_install_with_extras_editable_joined(script, data):
@@ -443,7 +443,7 @@ def test_install_with_extras_editable_joined(script, data):
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]')
assert script.site_packages / 'simple' in result.files_created
- assert script.site_packages / 'singlemodule.py'in result.files_created
+ assert script.site_packages / 'singlemodule.py' in result.files_created
def test_install_distribution_full_union(script, data):I had to fix a bunch of stuff and work around PyCQA/pycodestyle#935 |
|
Thanks @asottile for the diff. Upgrading to flake I will probably upgrade flake in a separate PR as well after fixing the issues with other files as per your diff. |
I completely agree. What I am thinking is that
Or If merging is only allowed for changes which don't affect the pip codebase a lot, I will make the flake8 changes here, and then this PR can be merged when the master opens for regular changes post 20.1.1 again. Let me know if you think is the best option. |
master branch is open; there's no reason to avoid-merging-PRs prior to a bugfix release anyway, since that will be based off the last tag, and not the master branch.
Merge now, and improve in a follow up sounds good to me! :) |
Thanks, I will create the new PR accordingly then from the master once this is merged. |
deveshks
force-pushed
the
remove-pretty-arg-from-mypy
branch
3 times, most recently
from
3b68f47 to
4d208b0
Compare
#8261 has been added to upgrade flake8. |
As mentioned in #8231 (comment), adding
--prettyto the mypy args make the mypy errors less readable.This PR upgrades mypy to v0.770 which fixes this issue in python/mypy#8145