Skip to content

bpo-39677: dis: rename the operand of MAKE_FUNCTION from argc to flags for 3.6+ #18550

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
merged 4 commits into from
Mar 14, 2020
Merged

bpo-39677: dis: rename the operand of MAKE_FUNCTION from argc to flags for 3.6+ #18550

merged 4 commits into from
Mar 14, 2020

Conversation

thautwarm
Copy link
Contributor

@thautwarm thautwarm commented Feb 18, 2020 •
edited by bedevere-bot
Loading

The semantics of MAKE_FUNCTION's operand has changed since Python 3.6, however the documentation kept its old name argc, however it now works the same way as other flags operands, hence now we change it to flags.

https://bugs.python.org/issue39677

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Feb 18, 2020
serhiy-storchaka
serhiy-storchaka reviewed Mar 14, 2020
View reviewed changes
Doc/library/dis.rst Outdated

Pushes a new function object on the stack. From bottom to top, the consumed
stack must consist of values if the argument carries a specified flag value
stack must consist of values if *flags* carries a specified flag value
Copy link
Member

Choose a reason for hiding this comment

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

I would keep this line as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have an idea if this should be called operand or argument, but we shall keep consistent in the whole Python documentations.
Just changed it according to your review. If you then think argument is not that proper just say something.

thautwarm and others added 2 commits March 14, 2020 22:00
@thautwarm @serhiy-storchaka
Update according to review
032179d 
Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
@thautwarm
Update according to review
0f36b72
@serhiy-storchaka
Copy link
Member

I proposed to return the original wording because "flags carries a specified flag value" looked slightly tautological to me.

The term operand is not used in the module documentation. If you think that it is more correct than argument please open a separate issue for discussion with English experts (not with me, my English is very poor).

I agree that the name of the argument was wrong. Thank you for your contribution!

@serhiy-storchaka serhiy-storchaka merged commit 6672c16 into python:master Mar 14, 2020
@miss-islington
Copy link
Contributor

Thanks @thautwarm for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thautwarm @serhiy-storchaka @miss-islington @the-knights-who-say-ni @bedevere-bot