gh-140764: Make "missed comma" syntax warning less cryptic #140893
Conversation
|
I was just working on this :) I was thinking of suggesting something like (as hinted): 1 Keep “perhaps” -> stays consistent with Python’s style. |
|
@alonme In the future, when someone says they are working on something in the issue thread, please don't open a PR. |
|
@StanFromIreland Sure, is there a rough guideline to how long after someone "claims" an issue is it okay to try and tackle it? |
I'd recommend you wait at least three weeks, you can always ask. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
This is not only for sequence elements. You get the same warning for set and dict which are not sequences. And for function arguments.
>>> {(1, 2) (3, 4)}
<python-input-0>:1: SyntaxWarning: 'tuple' object is not callable; perhaps you missed a comma?
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
{(1, 2) (3, 4)}
~~~~~~~^^^^^^
TypeError: 'tuple' object is not callable
>>> print((1, 2) (3, 4))
<python-input-1>:1: SyntaxWarning: 'tuple' object is not callable; perhaps you missed a comma?
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
print((1, 2) (3, 4))
~~~~~~~^^^^^^
TypeError: 'tuple' object is not callable
|
Given that mentioning "sequence elements" isn't applicable to all scenarios, how about we simply make it generic and end the warning with |
ncoghlan
left a comment
ncoghlan
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Several suggestions added to tweak the wording as per the comments from @serhiy-storchaka and @tadejmagajna, along with a corresponding adjustment to the news message.
(I'm skipping setting the "request changes" flag, as folks shouldn't feel obliged to wait for a second review specifically from me once the wording has been updated)
Lib/test/test_grammar.py
Outdated
| self.check_syntax_warning(test, msg) | ||
|
|
||
| msg=r'is not callable; perhaps you missed a comma\?' | ||
| msg=r'is not callable; did you miss a comma to separate sequence elements\?' |
There was a problem hiding this comment.
| msg=r'is not callable; did you miss a comma to separate sequence elements\?' | |
| msg=r'is not callable; did you miss a comma to separate elements\?' |
Lib/test/test_grammar.py
Outdated
| check('[t"x={x}" (3, 4)]') | ||
|
|
||
| msg=r'is not subscriptable; perhaps you missed a comma\?' | ||
| msg=r'is not subscriptable; did you miss a comma to separate sequence elements\?' |
There was a problem hiding this comment.
| msg=r'is not subscriptable; did you miss a comma to separate sequence elements\?' | |
| msg=r'is not subscriptable; did you miss a comma to separate elements\?' |
Lib/test/test_grammar.py
Outdated
| check('[t"x={x}" [i, j]]') | ||
|
|
||
| msg=r'indices must be integers or slices, not tuple; perhaps you missed a comma\?' | ||
| msg=r'indices must be integers or slices, not tuple; did you miss a comma to separate sequence elements\?' |
There was a problem hiding this comment.
| msg=r'indices must be integers or slices, not tuple; did you miss a comma to separate sequence elements\?' | |
| msg=r'indices must be integers or slices, not tuple; did you miss a comma to separate elements\?' |
| @@ -0,0 +1 @@ | |||
| Improve syntax warning messages for missing commas in sequences. The messages now say "did you miss a comma to separate sequence elements?" instead of "perhaps you missed a comma?". | |||
There was a problem hiding this comment.
| Improve syntax warning messages for missing commas in sequences. The messages now say "did you miss a comma to separate sequence elements?" instead of "perhaps you missed a comma?". | |
| Improve the suggestion in syntax warning messages for potentially missing commas when defining containers. The messages now say "did you miss a comma to separate elements?" instead of "perhaps you missed a comma?". |
Python/codegen.c
Outdated
| location loc = LOC(e); | ||
| return _PyCompile_Warn(c, loc, "'%.200s' object is not callable; " | ||
| "perhaps you missed a comma?", | ||
| "did you miss a comma to separate sequence elements?", |
There was a problem hiding this comment.
| "did you miss a comma to separate sequence elements?", | |
| "did you miss a comma to separate elements?", |
Python/codegen.c
Outdated
| location loc = LOC(e); | ||
| return _PyCompile_Warn(c, loc, "'%.200s' object is not subscriptable; " | ||
| "perhaps you missed a comma?", | ||
| "did you miss a comma to separate sequence elements?", |
There was a problem hiding this comment.
| "did you miss a comma to separate sequence elements?", | |
| "did you miss a comma to separate elements?", |
Python/codegen.c
Outdated
| return _PyCompile_Warn(c, loc, "%.200s indices must be integers " | ||
| "or slices, not %.200s; " | ||
| "perhaps you missed a comma?", | ||
| "did you miss a comma to separate sequence elements?", |
There was a problem hiding this comment.
| "did you miss a comma to separate sequence elements?", | |
| "did you miss a comma to separate elements?", |
|
Thanks @ncoghlan! |
6 participants
Uh oh!
There was an error while loading. Please reload this page.