bpo-45619: documentation of execution model: clarify and update binding summary #29232

Arthur-Milchior · 2021-10-27T02:32:14Z

bpo-45619: updating and improving the list of bindings

This does three changes that are quite related.

It breaks a long sentence enumerating things into a list
it adds two missing elements to the list:
a. assignment-expression can also bind a value. This should have been updated in 3.8.
b. variable binding can occur in pattern matching, an update of 3.10

This PR would be relevant in 3.8 and 3.9, except obviously for the pattern matching part. However, I understand that is not critical and can't be easily cherry-picked, so I assume it won't be backported.

https://bugs.python.org/issue45619

Doc/reference/executionmodel.rst

gvanrossum · 2021-10-27T04:16:10Z

Doc/reference/executionmodel.rst

+* class and function definitions (these bind the class or function name in the
+defining block),
+* and targets that are identifiers:
+  + if occurring in an assignment,


Honestly I think assignment expressions should be a separate bullet (as you said you considered in the PR description). Also, I think it should go above here, because the LHS of a walrus is not a target -- it's a plain name.

I do not know what is the exact definition of a target (even if I can guess from context), and I can't easily find such a definition in the documentation. If it is defined somewhere formally, I'd argue to link to it. Otherwise, seems a good place to define it.

You could link here for target: https://docs.python.org/3/reference/simple_stmts.html#grammar-token-python-grammar-target

Doc/reference/executionmodel.rst

Arthur-Milchior · 2021-10-27T13:12:35Z

Also, it will probably need the skip-news label. But I can't add it myself.

gvanrossum

I think we should make these tiny fixes and then consider it good enough to merge. I’ll wait a bit for others to pipe up though.

Doc/reference/executionmodel.rst

gvanrossum · 2021-10-27T14:07:44Z

Can you add the link to ‘target’?

gvanrossum · 2021-10-27T14:16:09Z

Can you fix the test errors? (You can run the doc build process locally to experiment until it passes. IIRC there’s a README in the Doc directory.)

Arthur-Milchior · 2021-10-27T14:25:24Z

I don't see how to add a link to target. For two reasons. First I need a plural and links seems to only work for singular. Secondly, the examples I find are keywords, but target is not a keyword. And since it seems that it is a Python specific role, I can't directly find a detailed description in sphinx documentation.

I did try to run. However,
make venv
make html
leads to

make html
mkdir -p build
Building NEWS from Misc/NEWS.d with blurb
PATH=./venv/bin:$PATH sphinx-build -b html -d build/doctrees -W . build/html
Running Sphinx v3.2.1

Warning, treated as error:
node class 'meta' is already registered, its visitors will be overridden
make: *** [Makefile:51: build] Error 2

The error message is really sparse, making it hard to actually debug. I'll however check the git error message hoping it will help understanding.

gvanrossum · 2021-10-27T14:32:54Z

Seems the docs pipeline is wedged due to a new release of docutils(?). Let’s forget about the link.

P.S. There is no need to squash commits, we do that at the end when we merge.

gvanrossum · 2021-10-27T15:44:27Z

If you merge/rebase the latest main, the doc build should be fixed. I'd like to make sure this passes before merging. Thanks!

gvanrossum · 2021-10-27T21:34:08Z

Doc/reference/executionmodel.rst

+  + in a capture pattern in structural pattern matching
+* :keyword:`import` statements.


I'm guessing you need a blank line after the interior bullet list? That might render ugly, but we can't keep the warning on the next line. But I'm not a ReST expert. :-(

Arthur-Milchior · 2021-10-28T01:53:36Z

I confirm that, after rebasing, I can now compile it. I'm new to Sphinx, but I'm confident I can now correct it quickly, thanks to clear error message

Arthur-Milchior · 2021-10-28T02:52:45Z

Done. The result looks like the attached image

and the link works. It goes to target the production rule. It may have been better that it go to the paragraph above, but honestly, I don't understand how this is supposed to work. I tried to search for example of reference to paragraph declared with index but I failed to find any reference between files that work.

Given that it still works and is a progress, I am confortable with merging it (being able to test the result also help a lot my confidence.)
In any case, it is not clear to me how I was supposed to use the link you gave me, given that Sphinx uses special syntax to create internal link.

terryjreedy · 2021-11-03T00:35:26Z

#29183 has this markup for a cross-doc link.

:meth:`x.__trunc__ <object.__trunc__>`

It links the library/math truncate entry to the reference/data_model object.trunc entry. This must depend on some global uniqueness.
@merwok Do you happen to know how to link target in this PR's revision to the reference execution-model chapter to https://docs.python.org/3/reference/simple_stmts.html#grammar-token-python-grammar-target?

merwok · 2021-11-03T01:36:35Z

That format is:

.. reference with custom text
:role:`free text <reference>`

.. short version, the text of the link is determined by the target:

:role:`reference`

reference must be a valid target, which can be created by various markup including .. _target: before a title (IIRC), .. class::, .. function:: and other similar directives, and .. index::. role is in general specific (class, func, meth, cmdoption but in truth most (all?) of these use a common namespace and so are exchangeable).

I hadn’t noticed the productionlist directive before this PR; documentation at https://devguide.python.org/documenting/#grammar-production-displays suggests that targets are created, but doesn’t make their format explicit, so I don’t know what target to use.

merwok · 2021-11-03T01:44:11Z

Found the reference: https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-productionlist

So it should be:

:token:`target` or :token:`targets <target>` for a plural

Arthur-Milchior · 2021-11-03T03:05:16Z

I just noticed I pushed two commits by accident. The first has nothing to do in this PR, trunc is already considered in another PR. Is it okay if I remove it and force push to keep only relevant commit ? Or do you want me to push a commit reverting the first commit ?

gvanrossum · 2021-11-03T03:09:58Z

For this situation a force-push is fine.

This does two changes that are quite related. 0. it add that variable binding can occur in pattern matching, an update of 3.10 which seems to have been omitted from the list of bindings 1. Given how long the sentence alrady was, with even subcases in the middle of the sentence, the commit breaks the sentence into an actual list.

Arthur-Milchior · 2021-11-03T03:36:30Z

Done. I removed the commit that should not have been here.

#29183 has this markup for a cross-doc link.
:meth:`x.__trunc__ <object.__trunc__>`
It links the library/math truncate entry to the reference/data_model object.trunc entry. This must depend on some global uniqueness. @merwok Do you happen to know how to link target in this PR's revision to the reference execution-model chapter to https://docs.python.org/3/reference/simple_stmts.html#grammar-token-python-grammar-target?

I suspect this is not relevant anymore for this PR.
It may have been relevant for d9c1868 that is already merged, and so may need another PR to update to the correct form if this one is not correct.

merwok · 2021-11-03T03:47:32Z

I am not so sure 🙂 that sub-discussion was prompted by this comment that’s relevant to this PR:

and the link works. It goes to target the production rule. It may have been better that it go to the paragraph above, but honestly, I don't understand how this is supposed to work. I tried to search for example of reference to paragraph declared with index but I failed to find any reference between files that work.

I replied to Terry’s question about linking to grammar tokens (re-discovering the markup that you had already used in fact), and can also answer your question:

:ref:`assignment`

.. or to control link text
:ref:`targets <assignment>`

should create a link to the heading you want. This line defines the target: https://github.com/python/cpython/blob/3.10/Doc/reference/simple_stmts.rst#L69

About index: they don’t reuse human-defined targets but create their own, such as https://docs.python.org/3.10/reference/simple_stmts.html#index-4 (example link reached from assignment expression [1] (click on [1]) on https://docs.python.org/3.10/genindex-A.html — I agree index is not the easiest directive to understand!)

gvanrossum · 2021-11-03T04:23:23Z

Hopefully @merwok and @terryjreedy can finish reviewing this PR? I need to focus elsewhere.

Arthur-Milchior · 2021-11-26T02:01:23Z

I just updated "targets" to be a link to the entry "assignment". I hope this ensure that the PR can now be approved. I'm not really sure whether "assignment" section or "token" literal is better; personally I'm totally fine with both. I used the latter because I thought it was what was asked, I beg your pardon for misunderstanding.

I edited the PR first message to update it with what is now actually in the PR.

gvanrossum

I think this is a sufficient improvement over what we have to merge the PR. However, I think this is an example of why our "reference" manual is too imprecise to be considered a language specification. Alas, it would take a committee of 10 people 10 years to come up with an adequate specification, and we just don't have those resources. So this will have to do.

miss-islington · 2021-11-26T05:19:09Z

Thanks @Arthur-Milchior for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2021-11-26T05:19:10Z

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

…ng summary (pythonGH-29232) This does two changes that are quite related. 0. it add that variable binding can occur in pattern matching, an update of 3.10 which seems to have been omitted from the list of bindings 1. Given how long the sentence already was, with even subcases in the middle of the sentence, the commit breaks the sentence into an actual list. (cherry picked from commit cd876c8) Co-authored-by: Arthur Milchior <[email protected]>

bedevere-bot · 2021-11-26T05:19:22Z

GH-29787 is a backport of this pull request to the 3.10 branch.

…ng summary (GH-29232) This does two changes that are quite related. 0. it add that variable binding can occur in pattern matching, an update of 3.10 which seems to have been omitted from the list of bindings 1. Given how long the sentence already was, with even subcases in the middle of the sentence, the commit breaks the sentence into an actual list. (cherry picked from commit cd876c8) Co-authored-by: Arthur Milchior <[email protected]>

…ng summary (python#29232) This does two changes that are quite related. 0. it add that variable binding can occur in pattern matching, an update of 3.10 which seems to have been omitted from the list of bindings 1. Given how long the sentence already was, with even subcases in the middle of the sentence, the commit breaks the sentence into an actual list.

bedevere-bot added the docs Documentation in the Doc dir label Oct 27, 2021

the-knights-who-say-ni added the CLA signed label Oct 27, 2021

bedevere-bot added the awaiting review label Oct 27, 2021

gvanrossum reviewed Oct 27, 2021

View reviewed changes

Arthur-Milchior force-pushed the case_binding branch from 7bf3fde to e650917 Compare October 27, 2021 13:11

gvanrossum reviewed Oct 27, 2021

View reviewed changes

Doc/reference/executionmodel.rst Outdated Show resolved Hide resolved

Doc/reference/executionmodel.rst Outdated Show resolved Hide resolved

gvanrossum changed the title ~~bpo-45619: documentation of execution model: pattern matching~~ bpo-45619: documentation of execution model: clarify and update binding summary Oct 27, 2021

gvanrossum added needs backport to 3.10 only security fixes skip news labels Oct 27, 2021

Arthur-Milchior force-pushed the case_binding branch from 17deea1 to a65420d Compare October 27, 2021 14:22

Arthur-Milchior force-pushed the case_binding branch from a65420d to a4d3902 Compare October 27, 2021 14:29

gvanrossum reviewed Oct 27, 2021

View reviewed changes

Arthur-Milchior force-pushed the case_binding branch from a4d3902 to 4d0ca93 Compare October 28, 2021 01:36

Arthur-Milchior force-pushed the case_binding branch from 4d0ca93 to fe1c060 Compare October 28, 2021 02:46

Arthur-Milchior force-pushed the case_binding branch from fd79da4 to fe1c060 Compare October 28, 2021 03:06

Arthur-Milchior force-pushed the case_binding branch from fe1c060 to ac31c78 Compare November 3, 2021 03:34

gvanrossum requested a review from terryjreedy November 3, 2021 04:22

gvanrossum requested a review from merwok November 3, 2021 04:23

Target becomes a reference to assignment

ed9b950

gvanrossum approved these changes Nov 26, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Nov 26, 2021

gvanrossum merged commit cd876c8 into python:main Nov 26, 2021

bedevere-bot removed the awaiting merge label Nov 26, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 26, 2021

Arthur-Milchior deleted the case_binding branch November 26, 2021 12:49

JelleZijlstra mentioned this pull request Apr 2, 2022

bpo-42128: Improve "Binding of names" doc section #25642

Closed

		+ in a capture pattern in structural pattern matching
		* :keyword:`import` statements.

Uh oh!

bpo-45619: documentation of execution model: clarify and update binding summary #29232

bpo-45619: documentation of execution model: clarify and update binding summary #29232

Uh oh!

Conversation

Arthur-Milchior commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

bpo-45619: updating and improving the list of bindings

Uh oh!

Uh oh!

Uh oh!

gvanrossum Oct 27, 2021

Choose a reason for hiding this comment

Uh oh!

Arthur-Milchior Oct 27, 2021

Choose a reason for hiding this comment

Uh oh!

gvanrossum Oct 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Arthur-Milchior commented Oct 27, 2021

Uh oh!

gvanrossum left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gvanrossum commented Oct 27, 2021

Uh oh!

gvanrossum commented Oct 27, 2021

Uh oh!

Arthur-Milchior commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Oct 27, 2021

Uh oh!

gvanrossum commented Oct 27, 2021

Uh oh!

gvanrossum Oct 27, 2021

Choose a reason for hiding this comment

Uh oh!

Arthur-Milchior commented Oct 28, 2021

Uh oh!

Arthur-Milchior commented Oct 28, 2021

Uh oh!

terryjreedy commented Nov 3, 2021

Uh oh!

merwok commented Nov 3, 2021

Uh oh!

merwok commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Arthur-Milchior commented Nov 3, 2021

Uh oh!

gvanrossum commented Nov 3, 2021

Uh oh!

Arthur-Milchior commented Nov 3, 2021

Uh oh!

merwok commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Nov 3, 2021

Uh oh!

Arthur-Milchior commented Nov 26, 2021

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Nov 26, 2021

Uh oh!

bedevere-bot commented Nov 26, 2021

Uh oh!

bedevere-bot commented Nov 26, 2021

Uh oh!

Uh oh!

Arthur-Milchior commented Oct 27, 2021 •

edited

Loading

gvanrossum left a comment •

edited

Loading

Arthur-Milchior commented Oct 27, 2021 •

edited

Loading

merwok commented Nov 3, 2021 •

edited

Loading

merwok commented Nov 3, 2021 •

edited

Loading