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.

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

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

Merged
merged 1 commit into from
Sep 24, 2021
Merged

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

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

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

@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
@@ -268,7 +268,7 @@ def iscode(object):
co_kwonlyargcount number of keyword only arguments (not including ** arg)
co_lnotab encoded mapping of line numbers to bytecode indices
co_name name with which this code object was defined
co_names tuple of names of local variables
co_names tuple of names of global variables
Copy link
Contributor

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

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

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

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

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

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

@csabella
Copy link
Contributor

@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

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

@jalexvig ping

@laike9m
Copy link
Contributor

laike9m commented Jul 23, 2020

Kindly ping

@jalexvig
Copy link
Contributor Author

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 added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Sep 23, 2021
@ambv ambv merged commit 3f8b23f into python:main Sep 24, 2021
@miss-islington
Copy link
Contributor

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

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

@bedevere-bot bedevere-bot removed needs backport to 3.10 only security fixes needs backport to 3.9 only security fixes labels Sep 24, 2021
@bedevere-bot
Copy link

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

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

@auvipy auvipy auvipy approved these changes

@bitdancer bitdancer bitdancer approved these changes

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.