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

[Reverted] Improved annotations for select.select() #1080

Merged
merged 2 commits into from
Mar 23, 2017

Conversation

nikhilm
Copy link
Contributor

@nikhilm nikhilm commented Mar 23, 2017

The TypeVars allow mypy to enforce that the returned lists are subsets of the arguments.

The TypeVars allow mypy to enforce that the returned lists are subsets of the arguments.
_X = TypeVar("_X")
def select(rlist: Sequence[_R], wlist: Sequence[_W], xlist: Sequence[_X],
timeout: float = ...) -> Tuple[List[_R],
List[_W],
Copy link
Member

Choose a reason for hiding this comment

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

Please fix up the indentation here.

@gvanrossum gvanrossum merged commit 7e05d47 into python:master Mar 23, 2017
@gvanrossum
Copy link
Member

Thanks!

@gvanrossum
Copy link
Member

Ow, this caused mypy's runtests.py to fail as follows:

PARALLEL 2
SUMMARY  139 tasks selected
passed 120, failed 1, pending 17; running 1                                     

FAILURE  #121 check stdlibsamples (3.2)

subprocess.py:1586: error: Need type annotation for variable
test/test_subprocess.py:1294: error: Need type annotation for variable

SUMMARY  1/139 tasks and 1/3981 tests failed
 FAILURE check stdlibsamples (3.2)
SUMMARY  1/139 tasks and 1/3981 tests failed
*** FAILURE ***

See discussion in #1059.

@gvanrossum
Copy link
Member

So the specific example was something like

rlist, wlist, xlist = select(rlist, wlist, [])

so mypy now complains that it doesn't have an item type for xlist.

I'm not sure how to resolve this -- reverting this PR feels sad, but that might be a common case (since few people even know what to do with xlist).

@gvanrossum
Copy link
Member

OTOH a simple solution would be to add casts to the sample code, e.g. select(rlist, wlist, cast(List, []). I'll try that.

gvanrossum pushed a commit to python/mypy that referenced this pull request Mar 24, 2017
gvanrossum added a commit to python/mypy that referenced this pull request Mar 24, 2017
This make mypy's runtests.py pass again with HEAD typeshed.

See python/typeshed#1080 (comment)
@JelleZijlstra
Copy link
Member

Hm, it seems like most code that uses select.select passes at least one empty list. I'd rather not force users to write a cast for each empty list argument, so maybe (unfortunately) it's better to revert.

@gvanrossum
Copy link
Member

But we could at least preserve the arguments as Sequence[Any].

@gvanrossum
Copy link
Member

BTW This is pretty common fallout from a sudden surge in contributions and nothing we should get too upset about. We just need to diligently fix the new issues.

@gvanrossum
Copy link
Member

Yeah, I'm going to revert this. Please try again with just the argument types.

gvanrossum pushed a commit that referenced this pull request Mar 25, 2017
@gvanrossum gvanrossum changed the title Improved annotations for select.select() [Reverted] Improved annotations for select.select() Mar 25, 2017
gvanrossum added a commit that referenced this pull request Mar 25, 2017
@nikhilm
Copy link
Contributor Author

nikhilm commented Mar 28, 2017

Thanks. will follow up in a few days.

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