Improved error message for incompatible default argument#3773
Conversation
|
I'd use "argument" instead of "parameter". With one exception we seem to
use "argument" everywhere when referring to function arguments, and
"parameter" for generic types.
|
|
Changed to |
|
I'm not sure why does travis fail for 3.3:
|
|
#3777 fixes the pytest error |
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
I like this, but I'd like to tweak the error message some more. I'd also like to see an explicit test for the Python 2 syntax where an argument can be a tuple, e.g.
def foo((x, y) = (1, 1)): passand some examples of errors there (an incorrect type, and too many or too few values).
| init = arg.initialization_statement | ||
| if init: | ||
| self.accept(init) | ||
| fmt = 'Incompatible default initializer for argument "{}"' |
There was a problem hiding this comment.
To shorten the message a bit, I'd suggest leaving out "initializer" here and replacing that word with "default" three lines below.
|
Also, if you rebase, the Travis error should be gone. |
|
Ping? |
|
Sorry. I've been working on this, but (as is usual for me) got carried away. What I have now is a check using an ad-hoc parameter passing instead, and a complete removal of I think these changes are nice, since they clean up some boilerplate, avoid using new I'm still trying to figure out how to take only what is needed, and how to get the parameter names correctly in the general case of tuple arguments (instead of having error messages about I can push the bigger change if you want to see it (it's bigger than necessary, but not huge), or continue to work on the smaller one, which was my intention. |
a374846 to
0413a0e
Compare
|
I'd appreciate it if you pushed just the wording changes I suggested, then
after that you can work on the bigger one and show it.
|
0413a0e to
2d03d48
Compare
2d03d48 to
6c89c13
Compare
|
I have actually pushed the wording changes, unless it's not what you meant: And the error message for tuples is not as clear but I think is passable: I also kept the use of call expression instead of assignment. I think it's better, but I can revert (this is not the big change). |
|
Sigh. You destroyed the github review history by squashing all your commits. I'm annoyed and am going to close this PR. Please start over with just what I requested without more refactoring. |
3 participants
When a default argument is incompatible with the parameter, the current error message is
Which is slightly confusing, and does not include the name of the parameter. I suggest changing it to something like
Although I'm sure there are better ways to phrase this.