-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch from attrs to dataclasses #1116
Conversation
6c76e58
to
c4a52e1
Compare
The main motivation here is that mypyc is going to have custom support for dataclasses but probably not attrs.
c4a52e1
to
b08d7d0
Compare
black.py
Outdated
mode.line_length, | ||
mode.string_normalization, | ||
is_pyi=True, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use dataclasses.replace(mode, is_pyi=True)
, which is kind of more idiomatic.
_for_loop_depths: List[int] = Factory(list) | ||
_lambda_argument_depths: List[int] = Factory(list) | ||
_for_loop_depths: List[int] = field(default_factory=list) | ||
_lambda_argument_depths: List[int] = field(default_factory=list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it makes sense to define a helper:
def Factory(fn):
return field(default_factory=fn)
to minimize the size of the diff? Not sure however what looks better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code is fine. I'd rather use dataclasses idiomatically than minimize diff size.
black.py
Outdated
@@ -629,7 +629,7 @@ def format_file_in_place( | |||
`mode` and `fast` options are passed to :func:`format_file_contents`. | |||
""" | |||
if src.suffix == ".pyi": | |||
mode = evolve(mode, is_pyi=True) | |||
mode = mode.replace(is_pyi=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's actually dataclasses.replace
, a global function (presumably so dataclasses don't inject a .replace
method everywhere)
black.py
Outdated
leaves: List[Leaf] = field(default_factory=list) | ||
comments: Dict[LeafID, List[Leaf]] = field( | ||
default_factory=dict | ||
) # keys ordered like `leaves` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put this comment on the previous line by itself instead? (I'd personally prefer if Black did that automatically in this case.)
setup.py
Outdated
@@ -42,7 +42,8 @@ def get_long_description() -> str: | |||
"typed-ast>=1.4.0", | |||
"regex", | |||
"pathspec>=0.6, <1", | |||
], | |||
] | |||
+ (["dataclasses>=0.6"] if sys.version_info < (3, 7) else []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer to do it like in python/typing@f254d69
The main motivation here is that mypyc is going to have custom support
for dataclasses but probably not attrs.