Skip to content

Silence modules in site-packages and typeshed - #5303

Merged
ilevkivskyi merged 7 commits into
python:masterfrom
emmatyping:silencepep561
Jul 6, 2018
Merged

Silence modules in site-packages and typeshed#5303
ilevkivskyi merged 7 commits into
python:masterfrom
emmatyping:silencepep561

Conversation

@emmatyping

@emmatyping emmatyping commented Jul 1, 2018

Copy link
Copy Markdown
Member

Fixes #5260 .

@emmatyping

emmatyping commented Jul 1, 2018 •
edited
Loading

Copy link
Copy Markdown
Member Author

Huh, this is strange:

from typing import Sequence
from os import fspath
s: Sequence[str]

s = tuple(map(fspath, s))
reveal_type(s)  # error: Revealed type is '<nothing>'

I suppose the map messes things up.

@ilevkivskyi

ilevkivskyi commented Jul 1, 2018

Copy link
Copy Markdown
Member

@ethanhs Is this a regression or it was always like this? (There is a chance this is related to a recent overload overhaul.)

@emmatyping

emmatyping commented Jul 1, 2018

Copy link
Copy Markdown
Member Author

Is this a regression or it was always like this?

Not sure, I created the repro with https://mypy-play.net, which uses 0.610, I haven't tested it on older versions.

@ilevkivskyi

ilevkivskyi commented Jul 1, 2018

Copy link
Copy Markdown
Member

No, it is a meet.is_overlapping_types problem which is still quite simplistic, it considers typing.Sequence[builtins.str] and builtins.tuple[builtins.str] non-overlapping (surprise).

The fix should be straightforward, but if you don't have time, just open an issue about this.

@emmatyping emmatyping mentioned this pull request Jul 2, 2018
Closed
This was referenced Jul 3, 2018
Closed
Closed
ilevkivskyi
ilevkivskyi reviewed Jul 5, 2018
View reviewed changes

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code looks good, but I think we should add a test that installs a PEP 561 with various errors (both import related and non-import-related) to be sure they are all silenced.

Comment thread test-data/unit/fine-grained-modules.test
main:2: error: Incompatible types in assignment (expression has type "int", variable has type "str")

[case testAddFileWhichImportsLibModuleWithErrors]
# flags: --no-silence-site-packages

@ilevkivskyi ilevkivskyi Jul 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this necessary here? Is broken installed as a PEP 561 package?

@emmatyping emmatyping Jul 5, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since opening this, I also implemented silencing typeshed, which broken is simulated as being part of via alt_lib_path.

Comment thread mypy/util.py Outdated

# backport commonpath for 3.4
if sys.platform == 'win32':
def commonpath(paths: Sequence[str]) -> str:

@ilevkivskyi ilevkivskyi Jul 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need the general algorithm here? As I understand we essentially only need something like is_sub_path(p1, p2). Not that I think it is important, but maybe it will allow to simplify code?

@emmatyping emmatyping Jul 5, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was originally hoping to eventually use os.path.commonpath once we drop support for 3.4, but I think you are right it would be better to do this ourselves. I will change this when I have time later.

@emmatyping

emmatyping commented Jul 5, 2018

Copy link
Copy Markdown
Member Author

I think we should add a test that installs a PEP 561 with various errors

I will add some errors to typedpkg to test the functionality.

@emmatyping emmatyping changed the title Silence modules in site-packages Silence modules in site-packages and typeshed Jul 5, 2018
@ilevkivskyi

ilevkivskyi commented Jul 5, 2018

Copy link
Copy Markdown
Member

I will add some errors to typedpkg to test the functionality.

OK, thanks! Just note that I would like to cut the release branch tomorrow morning. So it would be better to have the tests by that time.

@emmatyping

emmatyping commented Jul 5, 2018

Copy link
Copy Markdown
Member Author

Just note that I would like to cut the release branch tomorrow morning. So it would be better to have the tests by that time.

Sure thing, I will do that momentarily.

ilevkivskyi
ilevkivskyi approved these changes Jul 6, 2018
View reviewed changes

@ilevkivskyi ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, looks good now!

@ilevkivskyi
ilevkivskyi merged commit 1e7b517 into python:master Jul 6, 2018
@gvanrossum gvanrossum mentioned this pull request Jul 7, 2018
Closed
@emmatyping
emmatyping deleted the silencepep561 branch July 22, 2018 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ilevkivskyi ilevkivskyi ilevkivskyi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@emmatyping @ilevkivskyi