-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Use auto_attribs/native type hints for attrs classes. #11692
Changes from 1 commit
5b59b29
c232c20
057a770
b43094e
e475fd7
e1a778c
b5e7d38
117727a
d3af002
7d90348
be23f5d
93f31a5
80e3673
c7f8d89
6e0a95b
0ffe424
2da4cef
ddd20a4
ef6d2c1
9f114b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,22 +33,21 @@ | |
|
||
# This class can't be generic because it uses slots with attrs. | ||
# See: https://github.com/python-attrs/attrs/issues/313 | ||
@attr.s(slots=True) | ||
@attr.s(slots=True, auto_attribs=True) | ||
class DictionaryEntry: # should be: Generic[DKT, DV]. | ||
"""Returned when getting an entry from the cache | ||
|
||
Attributes: | ||
full: Whether the cache has the full or dict or just some keys. | ||
If not full then not all requested keys will necessarily be present | ||
in `value` | ||
known_absent: Keys that were looked up in the dict and were not | ||
there. | ||
known_absent: Keys that were looked up in the dict and were not there. | ||
value: The full or partial dict value | ||
""" | ||
|
||
full = attr.ib(type=bool) | ||
known_absent = attr.ib(type=Set[Any]) # should be: Set[DKT] | ||
value = attr.ib(type=Dict[Any, Any]) # should be: Dict[DKT, DV] | ||
full: bool | ||
known_absent: Set[Any] # should be: Set[DKT] | ||
value: Dict[Any, Any] # should be: Dict[DKT, DV] | ||
Comment on lines
-36
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. python-attrs/attrs#313 again! Argh. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now we're on 3.7 I wonder if we can use a stdlib dataclass here? We're not using any of the extra attrs machinery here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, but I'd rather that be left to a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed---just thinking out loud! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do this for a bunch of places so would probably be a reasonable change to make! |
||
|
||
def __len__(self) -> int: | ||
return len(self.value) | ||
|
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 wonder if this could just be
dict
now we're on 3.7+? Probably not worth changing.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 keeping it as
OrderedDict
is clearer for now!