Skip to content

Conversation

@pablogsal
Copy link
Member

@pablogsal pablogsal commented May 8, 2019

@pablogsal
Copy link
Member Author

CC: @pganssle @mariocj89

Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Grammar and style looks pretty much fine. Mainly the refcounts.dat file needs to be updated.

I will say that in the example of the four functions, I would probably group the "explanation of the function" parts a bit differently, because the explanation of the syntax is separated from the demonstration of the syntax, which means you have to either remember it perfectly or scroll up.

I'd rather see something like:

"A standard function definition does not restrict how arguments are passed, so they can be passed either by position or keyword:

>>> def standard_arg(arg):
 ...        print(arg)
>>> standard_arg(2)
2
>>> standard_arg(arg=2)
2

If an argument is specified to be positional only, it is an error to pass it by keyword:

>>> def pos_only_arg(arg, /):
...        print(arg)
>>> pos_only_arg(2)
2
>>> pos_only_arg(arg=2)
Traceback (most recent call last):
...

"

I understand that it's a trade-off, though. In your version, the contrast between the different syntaxes is highlighted, since all the definitions are right next to each other, whereas in my version the syntax is kept close to the explanation.

Copy link
Contributor

@mariocj89 mariocj89 left a comment

Choose a reason for hiding this comment

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

Looking great, some small optional nitpicks though I am not a native speaker :).

@pablogsal
Copy link
Member Author

@pganssle I think I prefer to highlight the contrast between the different syntaxes (and having the definition all together) more than how they are used (although is a very fine-grained detail). But I would like more opinions on this. @mariocj89 @willingc What do you think?


Looking at this in a bit more detail, it is possible to mark certain parameters
as *positional-only*. If *positional-only*, the parameters' order matters, and
the parameters cannot be passed by keyword. Positional-only parameters would
Copy link
Member

Choose a reason for hiding this comment

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

I think change "would be" to "are".


* Use positional-only if you want the name of the parameters to not be
available to the user. This is useful when parameter names have no real
meaning, if you want to enforce the order of the arguments when the function
Copy link
Contributor

@Carreau Carreau May 13, 2019

Choose a reason for hiding this comment

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

Or if you believe the name of the parameter may change in the future, and a change of parameter name can be considered a change in API ?

I've seen a couple of time def foo(filename) -> def foo(filename_or_buffer), which people don't realise is an api change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps at end of these bullets, add a bullet. For an API, use positional only to prevent breaking API changes if the parameter's name changes.

@vstinner
Copy link
Member

What is the status of this PR? Is it ready to be merged?

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

A few minor corrections/comments. Thanks @pablogsal 👍


* Use positional-only if you want the name of the parameters to not be
available to the user. This is useful when parameter names have no real
meaning, if you want to enforce the order of the arguments when the function
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps at end of these bullets, add a bullet. For an API, use positional only to prevent breaking API changes if the parameter's name changes.

@gvanrossum
Copy link
Member

What is this waiting for? It's documentation -- it's easy to iterate on.

Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>
pablogsal and others added 2 commits May 29, 2019 00:31
Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>
@willingc willingc merged commit b76302d into python:master May 28, 2019
@bedevere-bot
Copy link

@willingc: Please replace # with GH- in the commit message next time. Thanks!

@willingc
Copy link
Contributor

Thanks all for moving this forward. If there are additional doc changes, please file another PR. 🎉

@pablogsal pablogsal deleted the pep570_docs branch May 28, 2019 23:47
@pablogsal
Copy link
Member Author

pablogsal commented May 28, 2019

Thank you all for the review and the helpful comments and insights! ❤️

DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
python#13202)

* bpo-36540: Documentation for PEP570 - Python positional only arguments

* fixup! bpo-36540: Documentation for PEP570 - Python positional only arguments

* Update reference for compound statements

* Apply suggestions from Carol

Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>

* Update Doc/tutorial/controlflow.rst

Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>

* Add extra bullet point and minor edits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants