Skip to content

Conversation

sixolet
Copy link
Collaborator

@sixolet sixolet commented Nov 12, 2016

Introduces ARG_NAMED_OPT, a kind for optional keyword-only arguments.

@sixolet
Copy link
Collaborator Author

sixolet commented Nov 12, 2016

This should fix #2437

Naomi Seyfer added 2 commits November 14, 2016 10:56
Before, mypy would assume all keyword-only arguments were optional, and not
provide an error if you failed to provide a keyword-only argument at a call
site.  Now mypy provides you with a helpful error.
mypy/subtypes.py Outdated
@@ -1,4 +1,4 @@
from typing import List, Dict, Callable
from typing import List, Dict, Callable, Optional
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changes to this file seem to have no effect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, they must have snuck in from another part of the stacked diffs I'm managing.

mypy/strconv.py Outdated
a = self.func_helper(o)
a.insert(0, o.name())
if mypy.nodes.ARG_NAMED in [arg.kind for arg in o.arguments]:
arg_kinds = set([arg.kind for arg in o.arguments])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style nit: you can use a set comprehension here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot about those! Thank you.

mypy/strconv.py Outdated
a.insert(0, o.name())
if mypy.nodes.ARG_NAMED in [arg.kind for arg in o.arguments]:
arg_kinds = set([arg.kind for arg in o.arguments])
if len(arg_kinds & set([mypy.nodes.ARG_NAMED, mypy.nodes.ARG_NAMED_OPT])) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style nit: You can use a set literal here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And those!

main:3: error: Unexpected keyword argument "b"
main:4: error: Unexpected keyword argument "b"

[case testKeywordOnlyArguments]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a copy of this test case that uses the fast parser (# flags: --fast-parser in the test case).

f(A(), B()) # E: Too many positional arguments for "f"
g(A(), b=B())
g(b=B(), a=A())
g(A()) # E: Missing named argument "b" for function "g"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add test for multiple missing keyword-only arguments. Maybe also test a mix of optional and required keyword-only arguments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do.

Naomi Seyfer added 3 commits November 22, 2016 10:24
else:
output.append('{}:{}:{}: {}: {}'.format(
fnam, i + 1, col, severity, m.group('message')))
for possible_err_comment in input[i].split(' #'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be slightly better if this was ... input[i].split(' #')[1:]: so that the non-comment part of the line won't get processed (though this is unlikely to cause problems).

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 23, 2016

Can you look at the test failures? Otherwise looks good, thanks for the updates!

@JukkaL JukkaL merged commit fe1d523 into python:master Nov 24, 2016
@sixolet sixolet deleted the named-opt branch November 26, 2016 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants