Skip to content

Lots of stub fixes - #15

Merged
matthiaskramm merged 3 commits into
python:masterfrom
o11c:lots
Oct 20, 2015
Merged

Lots of stub fixes#15
matthiaskramm merged 3 commits into
python:masterfrom
o11c:lots

Conversation

@o11c

@o11c o11c commented Oct 16, 2015

Copy link
Copy Markdown
Contributor

This fixes a lot of stubs so that they actually typecheck.

The only thing I didn't fix was sqlalchemy, for which I filed #14

Of note:

  • Correct the name of python2 __builtin__ so that other code can import it.
  • Apply all updates from mypy stubs where they are newer.
  • Remove several functions from python2 stubs that only exist in python3.
  • Move several stubs to the correct directory (builtin/ vs stdlib/, 3 vs 3.4).
  • Use types instead of Any or nonexistent names like builtins.module.
  • Fix some incorrect use of Tuple[T] (fixed-length) and also some other sequences.
  • Remove a couple of other Any and unparameterized containers that I happened to see while applying other updates.
  • Make sure that builtins does not export typing names. There was a comment that implied that imported names were not reexported, but that is not what current tooling actually does, and there were several stubs as well as real-world code that used names like List and Dict without importing them.

What I did not do:

  • Verify that all the functions listed in stubs actually exist (I am working on a tool to do this automatically).
  • Verify that the types perfectly match (in particular, there are likely missing bytearray and other buffer cases).
  • Make any effort to deal with modules that exist in multiple python3 versions but with different contents.
  • Reason about platform-specific modules.
  • Touch the weird raise FooError code that I don't think belongs in stubs (and which is certainly incomplete, if not inaccurate, if it is intended to enumerate all the possible exceptions).

@o11c

o11c commented Oct 17, 2015

Copy link
Copy Markdown
Contributor Author

Note: with the enum commit, I have now tested this against all of: pypy2 (2.6), pypy3 (2.4), python3.2, python3.3, python3.4, and python3.5.

@matthiaskramm

matthiaskramm commented Oct 17, 2015

Copy link
Copy Markdown
Contributor

Thanks for the PR! Your changes look good (a few minor comments above).

Changing builtins.py to __builtin__.py sounds good to me, but it'll need a change in mypy as well. I'd recommend doing that in a separate PR, after python/mypy#884 is merged.

@o11c

o11c commented Oct 17, 2015

Copy link
Copy Markdown
Contributor Author

It doesn't need to wait, because a stub-override directory can take precedence.

I have stub-override/2.7/builtins.pyi as a symlink to typeshed/builtin/2.7/__builtin__.pyi, then stub-override/2.7/__builtin__.pyi as from builtins import *.

IMHO the right level of balance is: while typeshed should have consideration for implementations, it shouldn't make itself wrong for them.

@o11c

o11c commented Oct 17, 2015

Copy link
Copy Markdown
Contributor Author

I haven't rebased this yet because I'm still investigating a proper fix for the "don't export unless" import as problem.

@o11c

o11c commented Oct 17, 2015

Copy link
Copy Markdown
Contributor Author

If I don't finish this in an hour, I probably won't until Monday.

I discovered that mypy does have a remove_imported_names_from_symtable, but it seems ineffective and is wrong anyway.

@matthiaskramm

matthiaskramm commented Oct 17, 2015

Copy link
Copy Markdown
Contributor

No hurry, I won't get around to testing this PR with mypy before Monday, as well.

WRT __builtin__.pyi, let's just put a temporary symlink, builtins.pyi -> __builtin__.pyi into place in typeshed, to not make merging of python/mypy#884 more complicated than it needs to be.

@o11c

o11c commented Oct 19, 2015

Copy link
Copy Markdown
Contributor Author

A symlink is not enough, since that will result in duplicate definitions if anybody explicitly imports it.

I'll use the (working) glob import though.

@o11c

o11c commented Oct 20, 2015

Copy link
Copy Markdown
Contributor Author

Switching from my versions of import as fix to Jukka's was much more painful than I expected.

I have addressed all of the review comments now except for the typing.IO case, since everybody is forced to be wrong there, and the way I wrote it will make an eventual fix easier.

There's actually already a bug: python/typing#112

matthiaskramm added a commit that referenced this pull request Oct 20, 2015
@matthiaskramm
Merge pull request #15 from o11c/lots
ee1930d 
Lots of stub fixes
@matthiaskramm
matthiaskramm merged commit ee1930d into python:master Oct 20, 2015
@o11c o11c mentioned this pull request Oct 20, 2015
Merged
@o11c

o11c commented Oct 20, 2015

Copy link
Copy Markdown
Contributor Author

Note, the hang you experienced is a result of creating a subprocess.Popen with stdout=PIPE but not reading from it until the subprocess dies.

@o11c
o11c deleted the lots branch October 21, 2015 03:17
@JukkaL

JukkaL commented Oct 21, 2015

Copy link
Copy Markdown
Contributor

FYI, mypy should now deal with import as correctly in stubs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

3 participants

@o11c @matthiaskramm @JukkaL