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

Jump to bottom

bpo-37116: Use PEP 570 syntax for positional-only parameters. #13700

Merged
merged 13 commits into from Jun 1, 2019
Merged

bpo-37116: Use PEP 570 syntax for positional-only parameters. #13700

merged 13 commits into from Jun 1, 2019

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented May 31, 2019 •
edited by bedevere-bot

Based on #12620 but contains only changes that can be applied in 3.8. E.g. only getting rid of *args and name tricks and making self and cls positional-only.

https://bugs.python.org/issue37116

@serhiy-storchaka serhiy-storchaka changed the title Use PEP 570 syntax for positional-only parameters. bpo-37116: Use PEP 570 syntax for positional-only parameters. May 31, 2019
auvipy
auvipy approved these changes May 31, 2019
View changes
gvanrossum
gvanrossum approved these changes May 31, 2019
View changes
Copy link
Member

@gvanrossum gvanrossum left a comment

All LGTM. Very nice cleanups!

It looks like we technically have to add / whenever there's a **kwds (assuming the kwds is passed through and could contain any key)...

Lib/string.py
@@ -52,6 +52,8 @@ def capwords(s, sep=None):
import re as _re
from collections import ChainMap as _ChainMap

_sentinel_dict = {}
Copy link
Member

@gvanrossum gvanrossum May 31, 2019

Choose a reason for hiding this comment

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

Why not use object()? IIUC all the code below checks for its identity first and then substitutes another dict.

Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Jun 1, 2019

Choose a reason for hiding this comment

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

Because semantically the default value for the mapping parameter is an empty dict. The check mapping is _sentinel_dict is merely an optimization (we can avoid to create a ChainMap in this case).

Lib/weakref.py Show resolved Hide resolved
tirkarthi
tirkarthi reviewed May 31, 2019
View changes
Lib/unittest/case.py Show resolved Hide resolved
@pablogsal
Copy link
Member

pablogsal commented May 31, 2019

Thank you very much @serhiy-storchaka for working on this 🎉

tirkarthi
tirkarthi reviewed May 31, 2019
View changes
Lib/unittest/mock.py Show resolved Hide resolved
tirkarthi
tirkarthi reviewed May 31, 2019
View changes
Lib/unittest/mock.py Show resolved Hide resolved
ilevkivskyi
ilevkivskyi approved these changes May 31, 2019
View changes
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

typing/abc/collections code LGTM.

@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Jun 1, 2019 •
edited by bedevere-bot

Yes, we can add / in most cases whenever there's a **kwds.

With bpo-36518 we should not even add it if all positional arguments are required (this covers the majority of cases).

@serhiy-storchaka
Copy link
Member Author

serhiy-storchaka commented Jun 1, 2019

@pablogsal The two last bugs related to positional-only arguments I found when worked on this issue.

tirkarthi
tirkarthi approved these changes Jun 1, 2019
View changes
Copy link
Member

@tirkarthi tirkarthi left a comment

mock changes LGTM. There are few places where _mock_self is used in the async helpers later assigning to self following older pattern. I will try to change them in another PR so that this is good for beta 1. Thanks Serhiy.

@serhiy-storchaka serhiy-storchaka requested a review from a team as a code owner Jun 1, 2019
@serhiy-storchaka serhiy-storchaka merged commit 2085bd0 into python:master Jun 1, 2019
@serhiy-storchaka serhiy-storchaka deleted the use-pep-570-38 branch Jun 1, 2019
@bedevere-bot
Copy link

bedevere-bot commented Jun 1, 2019

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Windows7 3.x has failed when building commit 2085bd0.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/58/builds/2538) and take a look at the build logs.
  4. Check if the failure is related to this commit (2085bd0) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/58/builds/2538

Click to see traceback logs 
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_venv.py", line 327, in test_multiprocessing
    out, err = check_output([envpy, '-c',
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_venv.py", line 42, in check_output
    raise subprocess.CalledProcessError(
subprocess.CalledProcessError: Command '['d:\\temp\\tmpzz1z4c8u\\Scripts\\python_d.exe', '-c', 'from multiprocessing import Pool; print(Pool(1).apply_async("Python".lower).get(3))']' returned non-zero exit status 3221225477.

----------------------------------------------------------------------

Ran 16 tests in 144.483s

FAILED (errors=1, skipped=2)


Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_venv.py", line 327, in test_multiprocessing
    out, err = check_output([envpy, '-c',
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_venv.py", line 42, in check_output
    raise subprocess.CalledProcessError(
subprocess.CalledProcessError: Command '['d:\\temp\\tmp_qt0ywa6\\Scripts\\python_d.exe', '-c', 'from multiprocessing import Pool; print(Pool(1).apply_async("Python".lower).get(3))']' returned non-zero exit status 3221225477.

----------------------------------------------------------------------

Ran 16 tests in 65.325s

FAILED (errors=1, skipped=2)

DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@serhiy-storchaka @DinoV
bpo-37116: Use PEP 570 syntax for positional-only parameters. (python…
6ac94bc 
…GH-13700)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tirkarthi tirkarthi

@gvanrossum gvanrossum

@auvipy auvipy

@ilevkivskyi ilevkivskyi

@1st1 1st1

@ericvsmith ericvsmith

@rhettinger rhettinger

@terryjreedy terryjreedy

@python/email-team email-team

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@serhiy-storchaka @pablogsal @bedevere-bot @gvanrossum @tirkarthi @auvipy @ilevkivskyi @the-knights-who-say-ni