Skip to content

bpo-30951: Correct co_names documentation in inspect module #2743

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.

Sign up for GitHub

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
Merged
ambv merged 1 commit into from
Sep 24, 2021
Merged

bpo-30951: Correct co_names documentation in inspect module #2743

ambv merged 1 commit into from
Sep 24, 2021

Conversation

@jalexvig
Copy link
Contributor

@jalexvig jalexvig commented Jul 17, 2017 •
edited by bedevere-bot
Loading

Previously co_names was described as containing names of local variables. co_names however contains names of global variables (co_varnames contains local variable names). Documentation was updated to reflect this.

Relevant stackoverflow post:

https://stackoverflow.com/questions/45147260/what-is-co-names

https://bugs.python.org/issue30951

@the-knights-who-say-ni
Copy link

the-knights-who-say-ni commented Jul 17, 2017

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@mention-bot
Copy link

mention-bot commented Jul 17, 2017

@jalexvig, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Yhg1s, @1st1 and @zestyping to be potential reviewers.

@jalexvig jalexvig changed the title Correct co_names documentation in inspect module bpo-30951: Correct co_names documentation in inspect module Jul 17, 2017
marco-buttu
marco-buttu reviewed Jul 18, 2017
View reviewed changes
Lib/inspect.py Outdated
Copy link
Contributor

@marco-buttu marco-buttu Jul 18, 2017

Choose a reason for hiding this comment

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

What about tuple of names of global variables used in the bytecode?

Copy link
Contributor Author

@jalexvig jalexvig Jul 20, 2017

Choose a reason for hiding this comment

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

This description is currently under the code type... The other descriptions don't make explicit reference to bytecode (except for co_consts) since it is implied by the code type. Would be pretty repetitive to add reference to the bytecode in every description. Thoughts?

Copy link
Member

@bitdancer bitdancer Jul 20, 2017

Choose a reason for hiding this comment

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

The difference here is that "tuple of names of global variables" would be potentially misleading, since only those global variables used by the bytecode are included. (IIUC) This item is parallel to co_consts in that sense.

Copy link
Contributor

@marco-buttu marco-buttu Jul 20, 2017

Choose a reason for hiding this comment

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

Thanks @bitdancer, you pointed out perfectly my concern :-)

Copy link
Contributor Author

@jalexvig jalexvig Jul 20, 2017

Choose a reason for hiding this comment

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

Ah gotcha - updating.

@Mariatta Mariatta closed this Jul 5, 2018
@Mariatta Mariatta reopened this Jul 5, 2018
@serhiy-storchaka serhiy-storchaka mentioned this pull request Nov 23, 2018
Closed
@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Nov 23, 2018

co_names contains not only names of global variables. See my comment on the tracker.

@csabella
Copy link
Contributor

csabella commented May 17, 2019

@jalexvig, please take a look at incorporating the note from @serhiy-storchaka in your change. Thanks!

@csabella csabella added the stale Stale PR or inactive for long period of time. label Jan 20, 2020
@csabella
Copy link
Contributor

csabella commented Jan 20, 2020

Added the stale label as there hasn't been any response to the review comments.

@laike9m
Copy link
Contributor

laike9m commented Mar 23, 2020

So I'm guessing, co_names == all names in code - co_varnames - co_freevars ?

@smarie smarie mentioned this pull request Mar 23, 2020
Open
@csabella
Copy link
Contributor

csabella commented May 23, 2020

@jalexvig ping

@laike9m
Copy link
Contributor

laike9m commented Jul 23, 2020

Kindly ping

@jalexvig
Copy link
Contributor Author

jalexvig commented Aug 18, 2020

I haven't looked at this in a while, but updated with suggestion from Xavier Morel.

@ambv ambv added the skip news label Sep 23, 2021
@ambv
Copy link
Contributor

ambv commented Sep 23, 2021

Closing and re-opening to re-trigger CI.

@ambv ambv closed this Sep 23, 2021
@ambv ambv reopened this Sep 23, 2021
@ambv ambv merged commit 3f8b23f into python:main Sep 24, 2021
@miss-islington
Copy link
Contributor

miss-islington commented Sep 24, 2021

Thanks @jalexvig for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented Sep 24, 2021

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

@bedevere-bot
Copy link

bedevere-bot commented Sep 24, 2021

GH-28544 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 24, 2021
@jalexvig @miss-islington
bpo-30951: Correct co_names docstring in inspect module (pythonGH-2743)
38f4ef7 
(cherry picked from commit 3f8b23f)

Co-authored-by: Alex Vig <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 24, 2021
@jalexvig @miss-islington
bpo-30951: Correct co_names docstring in inspect module (pythonGH-2743)
2a3b63f 
(cherry picked from commit 3f8b23f)

Co-authored-by: Alex Vig <[email protected]>
ambv pushed a commit that referenced this pull request Sep 24, 2021
@miss-islington @jalexvig
bpo-30951: Correct co_names docstring in inspect module (GH-2743) (GH…
91a5ba1 
…-28544)

(cherry picked from commit 3f8b23f)

Co-authored-by: Alex Vig <[email protected]>
ambv pushed a commit that referenced this pull request Sep 24, 2021
@miss-islington @jalexvig
bpo-30951: Correct co_names docstring in inspect module (GH-2743) (GH…
402d5f3 
…-28543)

(cherry picked from commit 3f8b23f)

Co-authored-by: Alex Vig <[email protected]>
pablogsal pushed a commit that referenced this pull request Sep 29, 2021
@miss-islington @jalexvig @pablogsal
bpo-30951: Correct co_names docstring in inspect module (GH-2743) (GH…
7d652c1 
…-28543)

(cherry picked from commit 3f8b23f)

Co-authored-by: Alex Vig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bitdancer bitdancer bitdancer approved these changes

+2 more reviewers

@marco-buttu marco-buttu marco-buttu left review comments

@auvipy auvipy auvipy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.