[attrs plugin] Support kw_only=True by euresti · Pull Request #6107 · python/mypy
| auto_attribs: bool = ...) -> Callable[[_C], _C]: ... | ||
| auto_attribs: bool = ..., | ||
| kw_only: bool = ..., | ||
| cache_hash: bool = ..., |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that some of these are missing in typeshed. Could you please make a typeshed update PR?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that attrs includes stubs in the package maybe removing them from typeshed is better. python-attrs/attrs#480
| b = attr.ib("16") # type: str | ||
|
|
||
| B(b="foo", a=7) | ||
|
|
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty line is not needed.
| == | ||
| b.py:5: error: Non keyword-only attributes are not allowed after a keyword-only attribute. | ||
|
|
||
|
|
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer single empty line between tests.
| [builtins fixtures/attr.pyi] | ||
| [out] | ||
| == | ||
| b.py:5: error: Non keyword-only attributes are not allowed after a keyword-only attribute. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't test much of incremental logic. I would also add a test where base class in a.py is updated, while subclass is unchanged.
| init = _get_bool_argument(ctx, rvalue, 'init', True) | ||
| kw_only |= _get_bool_argument(ctx, rvalue, 'kw_only', False) | ||
| if kw_only and ctx.api.options.python_version[0] < 3: | ||
| ctx.api.fail("kw_only is not supported in Python 2", stmt) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would avoid repeating the same error message, and instead define a constant.
|
|
||
| # Read all the arguments from the call. | ||
| init = _get_bool_argument(ctx, rvalue, 'init', True) | ||
| kw_only |= _get_bool_argument(ctx, rvalue, 'kw_only', False) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this (a bit counterintuitive) behavior:
import attr @attr.s(kw_only=True) class A: a = attr.ib(kw_only=False) A(1) # Error
The logic here captures it correctly, but I would add a short comment that this is intentional.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _analyze_class(ctx: 'mypy.plugin.ClassDefContext', auto_attribs: bool) -> List[Attribute]: | ||
| def _analyze_class(ctx: 'mypy.plugin.ClassDefContext', | ||
| auto_attribs: bool, | ||
| kw_only: bool) -> List[Attribute]: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please document kw_only (and auto_attribs) in the docstring?