Skip to content

Remove Undefined from stubs #696

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

Merged
merged 3 commits into from
May 31, 2015
Merged

Conversation

JamesGuthrie
Copy link
Contributor

Part of #680

@JukkaL
Copy link
Collaborator

JukkaL commented May 25, 2015

Cool, thanks for the PR! A lot of "Python evaluation" tests (that use full stubs) are failing -- it could be an problem with builtins.py, typing.py or another stub file that is imported in many test cases. The stack traces are probably caused by a mypy bug -- I'll look at them.

By the way, are you working on other parts of #680? If you are not working on test cases, I'm probably going to start updating them to not use Undefined.

@JamesGuthrie
Copy link
Contributor Author

Huh, strange. It seems as though the travis.sh script runs more tests than python3 tests.py, so I didn't notice those failures. I will fix the following things:

  • It does look as though the tests will have to be updated because I removed Undefined = object() from the typing.py stub. I will put that definition back in typing.py so that the tests pass, we can remove it from the stubs when it's really no longer needed.
  • There are also other bugs that I introduced into the stubs which I'm seeing now thanks to the tests which are run in travis.sh. I will fix those and re-do this PR.
  • It looks as though ... is not valid python2, so I should remove x = ... # type: int in the 2.7 typing.py stub.

I haven't worked on anything else for #680, so I'm quite happy for you to continue working on it - I would like to get back to #638 at some point :)

@JukkaL
Copy link
Collaborator

JukkaL commented May 25, 2015

I removed most uses of Undefined in the rest of the codebase (outside stubs). Once this PR is in, we should be pretty close to getting rid of Undefined for good.

@JamesGuthrie JamesGuthrie force-pushed the remove-Undefined branch 2 times, most recently from 73f47f1 to 40d8e99 Compare May 26, 2015 08:37
@JamesGuthrie
Copy link
Contributor Author

These commits close #680 and include the latest version of typing.py from ambv/typehinting.

@JukkaL
Copy link
Collaborator

JukkaL commented May 27, 2015

Great, almost there!

I did a quick pass and found a few issues:

In stubs/2.7/difflib.pyi:

... a: Sequence[_T] = b: Sequence[_T], ... is wrong (near SequenceMatcher)

In stubs/3.2/_io.pyi:

_IOBase.seek has a missing =... default arg value:

-    def seek(self, offset, whence=Undefined): pass
+    def seek(self, offset, whence): pass

I found a few other instances where x=Undefined is replaced with just x. Probably worth manually checking all instances of =Undefined in the diff.

Also, the best replacement for an x=Undefined argument is x=... (with a literal ellipsis). This is slightly better than x=None, as sometimes None is not a valid argument value.

@JamesGuthrie
Copy link
Contributor Author

I fixed the two cases you mentioned above and went through all of the functions which were modified, ensuring that def f(x=Undefined) was replaced with def f(x=...), I managed to catch some other issues so I think it should all be good now.

@JukkaL JukkaL merged commit fd0d1af into python:master May 31, 2015
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.

2 participants