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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP 570: Positional-Only Arguments #6900

Merged
merged 4 commits into from May 31, 2019

Conversation

@brandtbucher
Copy link
Contributor

commented May 26, 2019

Fixes #6878. This was actually much more straightforward than I anticipated; since we already support __some forms of positional-only arguments, it was just a matter of adding a couple of lines to properly pick up the new posonlyargs attribute from the AST.

The tricky part is the tests. Travis doesn't currently have any build environments for 3.8.0a4 (even the python: "nightly" image is only 3.8.0a3+... 馃槓), so these tests fail with syntax errors. I've skipped them for now, but they pass fine on my local Python build. I added a small check in testcheck.py that will emit a warning when it's safe to unskip these tests.

@JukkaL
Copy link
Collaborator

left a comment

Thanks for implementing this! This gets us closer to Python 3.8 compatibility.

Looks good, just one idea for a test case.

f(arg="ERROR") # E: Unexpected keyword argument "arg" for "f"

[case testPEP570Signatures1-skip]
def f(p1: bytes, p2: float, /, p_or_kw: int, *, kw: str) -> None:

This comment has been minimized.

Copy link
@JukkaL

JukkaL May 30, 2019

Collaborator

Try also calling f with valid and invalid arguments. For example, make sure p1 is not a valid keyword argument but p_or_kw is both a valid positional and keyword argument, and kw must be a keyword argument.

@brandtbucher

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

Thanks for the suggestion @JukkaL! I've included the new test in my patch.

@JukkaL

JukkaL approved these changes May 31, 2019

Copy link
Collaborator

left a comment

Thanks for the update!

@JukkaL

This comment has been minimized.

Copy link
Collaborator

commented May 31, 2019

I verified manually that the tests pass on Python 3.8a4.

@JukkaL JukkaL merged commit 848a78d into python:master May 31, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@brandtbucher brandtbucher deleted the brandtbucher:pep570 branch May 31, 2019

ilevkivskyi added a commit that referenced this pull request Jun 2, 2019

Reenable PEP 570 tests. (#6923)
The `3.8-dev` Travis build environment now uses Python 3.8.0a4+!

This is left over from #6900. We aren't running 3.8 test right now for other reasons, but I've confirmed that these pass fine on my local build (@JukkaL confirmed too in the PR).

PattenR added a commit to PattenR/mypy that referenced this pull request Jun 23, 2019

PEP 570: Positional-Only Arguments (python#6900)
Fixes python#6878. This was actually much more straightforward than I 
anticipated; since we already support `__some` forms of 
positional-only arguments, it was just a matter of adding a couple 
of lines to properly pick up the new `posonlyargs` attribute from the 
AST.

Skip PEP 570 tests until Travis supports 3.8.0a4+.

PattenR added a commit to PattenR/mypy that referenced this pull request Jun 23, 2019

Reenable PEP 570 tests. (python#6923)
The `3.8-dev` Travis build environment now uses Python 3.8.0a4+!

This is left over from python#6900. We aren't running 3.8 test right now for other reasons, but I've confirmed that these pass fine on my local build (@JukkaL confirmed too in the PR).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.