bpo-38316: describe co_stacksize a little bit better #16983

isidentical · 2019-10-29T10:02:06Z

https://bugs.python.org/issue38316

Doc/reference/datamodel.rst

ammaraskar · 2019-11-15T18:47:11Z

hmm, now that I think about this more. I think more appropriate wording would be:

co_stacksize is the largest size the stack will grow to when executing the function;

isidentical · 2019-11-15T22:20:56Z

@ammaraskar can you check it now?

ammaraskar

LGTM with final suggestion

Doc/reference/datamodel.rst

ammaraskar · 2019-11-15T23:19:34Z

@vstinner Would you mind taking a look at this?

vstinner

Objects/frameobject.c allocates an array of code->co_stacksize + code->co_nlocals + ncells + nfrees items. The array size doesn't chagne during the execution of the function, it's not like x86 machine mode which manipulates the stack pointer (SP) register and use PUSH/POP.

:attr:co_stacksize is the required stack size (including local variables)

So this definition sounds wrong to me.

Instead, I suggest this definition:

:attr:`co_stacksize` is the required stack size

=> just remove (including local variables).

bedevere-bot · 2019-12-14T14:51:53Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.


        remove phrase

isidentical · 2019-12-15T06:41:56Z

I have made the requested changes; please review again

bedevere-bot · 2019-12-15T06:41:59Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

miss-islington · 2019-12-15T22:03:02Z

Thanks @isidentical for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2019-12-15T22:03:03Z

Thanks @isidentical for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington · 2019-12-15T22:03:08Z

Sorry, @isidentical and @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d587272fe3b0fcad2f23a490e76f9f82ca7d64ef 3.7

miss-islington · 2019-12-15T22:03:10Z

Sorry @isidentical and @vstinner, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker d587272fe3b0fcad2f23a490e76f9f82ca7d64ef 3.8

vstinner · 2019-12-15T22:04:00Z

@isidentical: The automated backport to 3.7 and 3.8 failed. Can you please try to manually backport these changes? Use " cherry_picker d587272 3.8" or "git cherry-pick -x d587272".

vstinner · 2019-12-18T20:45:08Z

Ping @isidentical, tell me if you prefer that I do the backport.

isidentical · 2019-12-19T14:06:19Z

Thanks for the ping @vstinner, I totally forgot backports. Sending them right away


        [3.7] bpo-38316: Fix co_stacksize documentation (pythonGH-16983).

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>

bedevere-bot · 2019-12-19T14:36:09Z

GH-17660 is a backport of this pull request to the 3.7 branch.


        [3.8] bpo-38316: Fix co_stacksize documentation (pythonGH-16983)

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>

bedevere-bot · 2019-12-19T14:38:41Z

GH-17661 is a backport of this pull request to the 3.8 branch.


        [3.7] bpo-38316: Fix co_stacksize documentation (GH-16983). (GH-17660)

@vstinner

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com> https://bugs.python.org/issue38316 Automerge-Triggered-By: @vstinner


        [3.8] bpo-38316: Fix co_stacksize documentation (GH-16983) (GH-17661)

@vstinner

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com> https://bugs.python.org/issue38316 Automerge-Triggered-By: @vstinner

the-knights-who-say-ni added the CLA signed label Oct 29, 2019

bedevere-bot added type-documentation awaiting review labels Oct 29, 2019

isidentical force-pushed the isidentical:bpo-38316 branch from 92abd96 to 728a886 Oct 29, 2019

ammaraskar added the skip news label Oct 29, 2019

ammaraskar reviewed Oct 29, 2019

View changes

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

isidentical force-pushed the isidentical:bpo-38316 branch from 728a886 to 95c86b9 Oct 30, 2019

isidentical requested a review from ammaraskar Nov 15, 2019

isidentical force-pushed the isidentical:bpo-38316 branch from 95c86b9 to 09b25b1 Nov 15, 2019

ammaraskar approved these changes Nov 15, 2019

View changes

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

bedevere-bot added awaiting core review and removed awaiting review labels Nov 15, 2019

vstinner requested changes Dec 14, 2019

View changes

bedevere-bot removed the awaiting core review label Dec 14, 2019

bedevere-bot added the awaiting changes label Dec 14, 2019

remove phrase

Loading status checks…

da1ab34

isidentical force-pushed the isidentical:bpo-38316 branch from abbc1f1 to da1ab34 Dec 15, 2019

bedevere-bot added awaiting change review and removed awaiting changes labels Dec 15, 2019

bedevere-bot requested a review from vstinner Dec 15, 2019

bedevere-bot removed the awaiting change review label Dec 15, 2019

vstinner added the needs backport to 3.7 label Dec 15, 2019

vstinner added the needs backport to 3.8 label Dec 15, 2019

miss-islington assigned vstinner Dec 15, 2019

isidentical added a commit to isidentical/cpython that referenced this pull request Dec 19, 2019

[3.7] bpo-38316: Fix co_stacksize documentation (pythonGH-16983).

0973c24

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>

bedevere-bot removed the needs backport to 3.7 label Dec 19, 2019

isidentical added a commit to isidentical/cpython that referenced this pull request Dec 19, 2019

[3.8] bpo-38316: Fix co_stacksize documentation (pythonGH-16983)

60e6279

(cherry picked from commit d587272) Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>

bedevere-bot removed the needs backport to 3.8 label Dec 19, 2019

isidentical deleted the isidentical:bpo-38316 branch Dec 19, 2019

python/cpython

bpo-38316: describe co_stacksize a little bit better #16983

bpo-38316: describe co_stacksize a little bit better #16983

isidentical commented Oct 29, 2019 •

edited by bedevere-bot

This comment has been minimized.

ammaraskar commented Nov 15, 2019

This comment has been minimized.

isidentical commented Nov 15, 2019

ammaraskar left a comment

This comment has been minimized.

ammaraskar commented Nov 15, 2019

vstinner left a comment

This comment has been minimized.

bedevere-bot commented Dec 14, 2019

This comment has been minimized.

isidentical commented Dec 15, 2019

This comment has been minimized.

bedevere-bot commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

vstinner commented Dec 15, 2019

This comment has been minimized.

vstinner commented Dec 18, 2019

This comment has been minimized.

isidentical commented Dec 19, 2019

This comment has been minimized.

bedevere-bot commented Dec 19, 2019

This comment has been minimized.

bedevere-bot commented Dec 19, 2019

Sponsor python/cpython

python/cpython

Join GitHub today

bpo-38316: describe co_stacksize a little bit better #16983

bpo-38316: describe co_stacksize a little bit better #16983

Conversation

isidentical commented Oct 29, 2019 • edited by bedevere-bot

This comment has been minimized.

ammaraskar commented Nov 15, 2019

This comment has been minimized.

isidentical commented Nov 15, 2019

ammaraskar left a comment

This comment has been minimized.

ammaraskar commented Nov 15, 2019

vstinner left a comment

This comment has been minimized.

bedevere-bot commented Dec 14, 2019

This comment has been minimized.

isidentical commented Dec 15, 2019

This comment has been minimized.

bedevere-bot commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

miss-islington commented Dec 15, 2019

This comment has been minimized.

vstinner commented Dec 15, 2019

This comment has been minimized.

vstinner commented Dec 18, 2019

This comment has been minimized.

isidentical commented Dec 19, 2019

This comment has been minimized.

bedevere-bot commented Dec 19, 2019

This comment has been minimized.

bedevere-bot commented Dec 19, 2019

isidentical commented Oct 29, 2019 •

edited by bedevere-bot