plugins: delete open plugin #9275

hauntsaninja · 2020-08-07T04:33:28Z

Improvements to typeshed types using literals should have rendered this
unnecessary.

hauntsaninja · 2020-08-07T04:33:40Z

Draft PR because we'll need python/typeshed#4407 to be merged and synced before tests will pass.

gvanrossum

I expect this is going to make a lot of people unhappy — being more strict it will cause new failures in people’s CI. @JukkaL what do you think? Would it be bad for the Dropbox build?

gvanrossum · 2020-08-07T04:42:54Z

test-data/stdlib-samples/3.2/test/test_genericpath.py

-        finally:
-            if not f.closed:
-                f.close()
-            support.unlink(support.TESTFN)


Is it safe to drop the finally clause with unlink?

It wouldn't be, if this code were ever executed! We've talked about entirely getting rid of the stdlib samples before (e.g., in #8838), so I didn't try very hard to preserve them.

JukkaL · 2020-08-07T09:55:50Z

@JukkaL what do you think? Would it be bad for the Dropbox build?

It's unclear, but I wouldn't be surprised if this requires quite a few changes, as there are thousands of calls to open. I can test this against Dropbox internal repos once the build is green.

JelleZijlstra · 2020-08-07T13:36:48Z

mypy/report.py

@@ -243,10 +243,10 @@ def _write_out_report(self,
            f.write(separator + '\n')
            for row_values in rows:
                r = ("{:>{}}" * len(widths)).format(*itertools.chain(*zip(row_values, widths)))
-                f.writelines(r + '\n')
+                f.write(r + '\n')


What made this change necessary?

That looks like it fixes a bug! writelines takes a sequence of strings, not a single string (though because a string is a sequence of strings and writelines just writes the concatenation of the items it worked :-).

Though I'm not sure how the change found this bug -- does the TextIoWriter class (or whatever is used) not have writelines?

We now infer f as TextIOWrapper. TextIOWrapper and TextIOBase take List[str] for writelines in typeshed. Maybe that signature should be loosened though.

Yes, I think that should be Iterable[str]. I just tried in 3.6 and TextIOWrapper.writelines accepts at least tuples and generator expressions.

And it looks like the perennial "str is a iterable of str" bug is benign in this case, so there's not really any downside to the looser signature.

hauntsaninja · 2020-08-22T05:19:27Z

All green now! I'm not sure what will become of this PR, but it's ready to find out!

hauntsaninja · 2020-09-26T01:25:40Z

I wrote a script that democratises the ability to evaluate changes like this: https://github.com/hauntsaninja/mypy_primer

hauntsaninja · 2020-09-26T01:28:19Z

Here are the results for this PR:

Fixed a false positive:
- zerver/lib/cache.py:97: error: Incompatible types in assignment (expression has type "TextIO", variable has type "TextIOWrapper")  [assignment]

Exposes an incorrect type in typeshed:
+ sphinx/ext/coverage.py:250:38: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "Generator[str, None, None]"; expected "List[str]"
+ sphinx/ext/coverage.py:279:46: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "Generator[str, None, None]"; expected "List[str]"
+ sphinx/ext/coverage.py:297:30: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "Generator[str, None, None]"; expected "List[str]"

Finds a "bug" (but also would go away by loosening the above type in typeshed):
+ mypy/report.py:246: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "str"; expected "List[str]"  [arg-type]
+ mypy/report.py:249: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "str"; expected "List[str]"  [arg-type]
+ tools/lib/provision_inner.py:79: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "str"; expected "List[str]"  [arg-type]
+ tools/lib/provision_inner.py:82: error: Argument 1 to "writelines" of "TextIOWrapper" has incompatible type "str"; expected "List[str]"  [arg-type]

Presumed false positive (although intentional on mypy's part, in the absence of --allow-redefinition):
+ src/pip/_internal/req/req_uninstall.py:644: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")
+ src/pip/_internal/operations/install/wheel.py:126: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")
+ mypyc/build.py:304: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")  [assignment]
+ tools/setup/generate_integration_bots_avatars.py:52: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")  [assignment]
+ pandas/io/stata.py:1063: error: Incompatible types in assignment (expression has type "BytesIO", variable has type "BufferedReader")  [assignment]

If we loosen the type of writelines in typeshed (as we should), this brings the count to five errors against (one of which is fixed in this PR) and one error for out of a couple million lines of code. The five errors against are arguably "by design", since --allow-redefinition is not the default.

Improvements to typeshed types using literals should have rendered this unnecessary.

Since errors shouldn't be used with binary modes. Maybe should just delete?

This came up in python/mypy#9275

This came up in python/mypy#9275 Co-authored-by: hauntsaninja <>

msullivan · 2020-10-18T03:37:19Z

There's a competing "delete open plugin" from earlier: #7794.

Do you have any opinions on this one versus that, @hauntsaninja

hauntsaninja · 2020-10-18T06:01:07Z

Yeah, I noticed the duplication a couple weeks ago when cleaning up some old PRs :-/

The big difference is that that PR no longer gets rid of the open plugin, because of https://github.com/python/mypy/pull/7794/files#discussion_r342519954
My opinion is that raising an error for the errors='replace' + binary case doesn't pull the extra weight of having a plugin.

It's also not clear to me why that PR doesn't necessitate some of the changes that I've made here. It's possible merging in a newer typeshed into that PR would surface those complaints (although I'll note that I changed the writelines type in typeshed in python/typeshed#4642)

msullivan · 2020-10-18T20:21:29Z

@JukkaL @ilevkivskyi what is the damage like for this PR at Dropbox?

github-actions · 2022-02-09T05:37:45Z

Diff from mypy_primer, showing the effect of this PR on open source code:

mypy_primer (https://github.com/hauntsaninja/mypy_primer)
+ mypy_primer.py:118: error: Unused "type: ignore" comment
+ mypy_primer.py:119: error: Item "None" of "Optional[bytes]" has no attribute "count"

mypy (https://github.com/python/mypy)
+ mypyc/build.py:311: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")  [assignment]

pip (https://github.com/pypa/pip)
+ src/pip/_internal/req/req_uninstall.py:623: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")
+ src/pip/_internal/operations/install/wheel.py:107: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")

zulip (https://github.com/zulip/zulip)
+ tools/setup/generate_integration_bots_avatars.py:54: error: Incompatible types in assignment (expression has type "BufferedWriter", variable has type "BufferedReader")  [assignment]

hauntsaninja · 2022-02-09T06:29:43Z

Issues that this would fix are reported fairly regularly. Given how long this PR has been lying around, and that the mypy_primer output doesn't look unworkable, I'm going to try asking for forgiveness on this merge :-)

Upgrade mypy to 0.971, which is the last version that supports Python 3.6 (the oldest Python version that we currently run on the CI). This fixes the error ``` tests/scripts/analyze_outcomes.py:117: error: Incompatible return value type (got "IO[Any]", expected "TextIO") tests/scripts/analyze_outcomes.py:119: error: Incompatible return value type (got "IO[Any]", expected "TextIO") ``` As far as I can tell the fix is python/mypy#9275 which was released in mypy 0.940. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Upgrade mypy to 0.971, which is the last version that supports Python 3.6 (the oldest Python version that we currently run on the CI). This fixes the error ``` framework/scripts/mbedtls_framework/outcome_analysis.py:119: error: Incompatible return value type (got "IO[Any]", expected "TextIO") framework/scripts/mbedtls_framework/outcome_analysis.py:121: error: Incompatible return value type (got "IO[Any]", expected "TextIO") ``` As far as I can tell the fix is python/mypy#9275 which was released in mypy 0.940. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gvanrossum reviewed Aug 7, 2020

View reviewed changes

JelleZijlstra reviewed Aug 7, 2020

View reviewed changes

hauntsaninja force-pushed the opplug branch from 9f2d7ce to 7fc5e32 Compare August 22, 2020 03:50

hauntsaninja marked this pull request as ready for review August 22, 2020 03:51

hauntsaninja force-pushed the opplug branch from 7fc5e32 to 331684c Compare August 22, 2020 03:52

This was referenced Sep 26, 2020

[FYI] mypy_primer #9490

Closed

[FYI] mypy_primer python/typeshed#4580

Closed

hauntsaninja force-pushed the opplug branch from e0f4960 to 98033ac Compare September 26, 2020 21:10

hauntsaninja added 5 commits September 27, 2020 17:04

plugins: delete open plugin

027ba3b

Improvements to typeshed types using literals should have rendered this unnecessary.

update tests

6ac771c

fix ensuing type errors

e7061ef

update pathlib tests

d2b37c7

fix pathlib tests to not use binary mode

8502699

Since errors shouldn't be used with binary modes. Maybe should just delete?

hauntsaninja force-pushed the opplug branch from 98033ac to 8502699 Compare September 28, 2020 00:06

hauntsaninja pushed a commit to hauntsaninja/typeshed that referenced this pull request Oct 10, 2020

io: loosen writelines type to iterable

407a1ae

This came up in python/mypy#9275

hauntsaninja mentioned this pull request Oct 10, 2020

io: loosen writelines type to iterable python/typeshed#4642

Merged

hauntsaninja added a commit to python/typeshed that referenced this pull request Oct 10, 2020

io: loosen writelines type to iterable (#4642)

228f74d

This came up in python/mypy#9275 Co-authored-by: hauntsaninja <>

msullivan mentioned this pull request Oct 18, 2020

Remove open plugin, not needed with overloaded signatures on typeshed #7794

Closed

arnimarj mentioned this pull request Nov 1, 2020

plugin causes open() signature to change Shoobx/mypy-zope#16

Closed

JelleZijlstra mentioned this pull request Apr 1, 2021

File streams: "BufferedWriter" has incompatible type "BinaryIO"; expected "RawIOBase" #10271

Closed

Akuli mentioned this pull request May 18, 2021

Fallback overloads for literal argument python/typeshed#5493

Open

Merge branch 'master' into opplug

3c5c388

Merge branch 'master' into opplug

ec809bf

This comment has been minimized.

Sign in to view

Akuli mentioned this pull request Sep 25, 2021

mypy does not honour open() overrides from typeshed. #11193

Closed

hauntsaninja linked an issue Sep 25, 2021 that may be closed by this pull request

mypy does not honour open() overrides from typeshed. #11193

Closed

emmatyping mentioned this pull request Oct 19, 2021

Release 0.920 planning #11158

Closed

Merge remote-tracking branch 'upstream/master' into opplug

d5998e7

hauntsaninja merged commit 0647570 into python:master Feb 9, 2022

hauntsaninja deleted the opplug branch February 9, 2022 06:30

gvanrossum mentioned this pull request Mar 1, 2022

Introduce typing.STRICTER_STUBS python/typing#1096

Open

Michael0x2a mentioned this pull request Sep 2, 2022

Use of built-in open() causes crash if file named io.py #10990

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plugins: delete open plugin #9275

plugins: delete open plugin #9275

hauntsaninja commented Aug 7, 2020

hauntsaninja commented Aug 7, 2020

gvanrossum left a comment

gvanrossum Aug 7, 2020

hauntsaninja Aug 7, 2020 •

edited

Loading

JukkaL commented Aug 7, 2020

JelleZijlstra Aug 7, 2020

gvanrossum Aug 7, 2020

hauntsaninja Aug 7, 2020

JelleZijlstra Aug 7, 2020

hauntsaninja Aug 7, 2020

hauntsaninja commented Aug 22, 2020

hauntsaninja commented Sep 26, 2020

hauntsaninja commented Sep 26, 2020 •

edited

Loading

msullivan commented Oct 18, 2020

hauntsaninja commented Oct 18, 2020 •

edited

Loading

msullivan commented Oct 18, 2020

This comment has been minimized.

github-actions bot commented Feb 9, 2022

hauntsaninja commented Feb 9, 2022

plugins: delete open plugin #9275

plugins: delete open plugin #9275

Conversation

hauntsaninja commented Aug 7, 2020

hauntsaninja commented Aug 7, 2020

gvanrossum left a comment

Choose a reason for hiding this comment

gvanrossum Aug 7, 2020

Choose a reason for hiding this comment

hauntsaninja Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

JukkaL commented Aug 7, 2020

JelleZijlstra Aug 7, 2020

Choose a reason for hiding this comment

gvanrossum Aug 7, 2020

Choose a reason for hiding this comment

hauntsaninja Aug 7, 2020

Choose a reason for hiding this comment

JelleZijlstra Aug 7, 2020

Choose a reason for hiding this comment

hauntsaninja Aug 7, 2020

Choose a reason for hiding this comment

hauntsaninja commented Aug 22, 2020

hauntsaninja commented Sep 26, 2020

hauntsaninja commented Sep 26, 2020 • edited Loading

msullivan commented Oct 18, 2020

hauntsaninja commented Oct 18, 2020 • edited Loading

msullivan commented Oct 18, 2020

This comment has been minimized.

github-actions bot commented Feb 9, 2022

hauntsaninja commented Feb 9, 2022

hauntsaninja Aug 7, 2020 •

edited

Loading

hauntsaninja commented Sep 26, 2020 •

edited

Loading

hauntsaninja commented Oct 18, 2020 •

edited

Loading