bpo-29622: make AST constructor accepts less than enough number of positional arguments by methane · Pull Request #249 · python/cpython (original) (raw)
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.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Currently, AST constructor accepts
a. empty arguments
b. positional arguments, only when it's length is exactly same to number of fields
c. keyword arguments. No check for missing required fields.
Only (b) is strict. And it require argument even if matching field is optional.
This pull request removes the strict check.
Missing required field can be detected when compiling AST though.
| @@ -666,11 +666,10 @@ def visitModule(self, mod): |
|---|
| } |
| res = 0; /* if no error occurs, this stays 0 to the end */ |
| if (PyTuple_GET_SIZE(args) > 0) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is redundant.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
bpo-29622: Loosen restriction of number of positional argument of AST constructor bpo-29622: Make trailing optional fields to optional arguments of AST type
I implemented minimum number of arguments.
But I still don't know should I check it, since AST constructor doesn't check it when keyword argument is used.
Could I ask that we don't make arguments required, even if you need them to construct a valid AST? I have a pair of little side projects (astsearch and astcheck) which rely on the ability to instantiate incomplete AST objects. I reuse these as templates, so you can check for things like 'for loop with an else clause' without specifying any of the required pieces of a for loop.
Obviously this isn't what the AST classes are intended for, but it works pretty nicely. And yes, I am being this guy:
@takluyver Thank you. I decided to revert last commit.
bpo-29622: Make trailing optional fields to optional arguments of AST type bpo-29622: make AST constructor accept less than enough number of positional arguments
bpo-29622: make AST constructor accept less than enough number of positional arguments bpo-29622: make AST constructor accepts less than enough number of positional arguments
| @@ -373,12 +373,8 @@ def test_nodeclasses(self): |
|---|
| self.assertEqual(x.right, 3) |
| self.assertEqual(x.lineno, 0) |
| # node raises exception when not given enough arguments |
| self.assertRaises(TypeError, ast.BinOp, 1, 2) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep these and change to assertNotRaises ?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, too late, that was merged while I was reviewing :-)
| for (i = 0; i < PyTuple_GET_SIZE(args); i++) { |
|---|
| /* cannot be reached when fields is NULL */ |
| PyObject *name = PySequence_GetItem(fields, i); |
| if (!name) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity what's the prefered python style !variable or == NULL
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't care about !name or name == NULL. And I only removed indent.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I saw. It was a genuine question like if you are coding something new, what's the prefered style in the CPython codebase?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
akruis pushed a commit to akruis/cpython that referenced this pull request
…n API
Stackless does not support custom frame evaluation functions defined by PEP 523. If an extension module sets a custom frame evaluation function, Stackless now terminates to prevent undefined behavior.
(cherry picked from commit d57b317)
akruis pushed a commit to akruis/cpython that referenced this pull request
…n API
Stackless does not support custom frame evaluation functions defined by PEP 523. If an extension module sets a custom frame evaluation function, Stackless now terminates to prevent undefined behavior.
jaraco pushed a commit that referenced this pull request
jaraco added a commit to jaraco/cpython that referenced this pull request
…athsep
Honor '/'-separated names in ResourceContainer.joinpath.