Skip to content

Ensure required keyword-only arguments are provided.#2441

Merged
JukkaL merged 7 commits intopython:masterfrom
sixolet:named-opt
Nov 24, 2016
Merged

Ensure required keyword-only arguments are provided.#2441
JukkaL merged 7 commits intopython:masterfrom
sixolet:named-opt

Conversation

@sixolet
Copy link
Copy Markdown
Collaborator

@sixolet sixolet commented Nov 12, 2016

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

@sixolet
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

And those!

@@ -115,13 +115,20 @@ main:4: error: Unexpected keyword argument "b"
[case testKeywordOnlyArguments]
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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