Make Node a dataclass#5
Conversation
| vs = x_to_read_quorum_vars[x] | ||
| x_load += fr * sum(vs) / self.node(x).read_capacity | ||
| xread_capacity = self.node(x).read_capacity | ||
| if xread_capacity is None: |
There was a problem hiding this comment.
This code would not be necessary if we made:
read_capacity: float = 1.0
in the dataclass. More instances below.
|
Ah, I've been targeting Python 3.6, and I think dataclasses are a 3.7+ thing? |
|
Backports are available: https://pypi.org/project/dataclasses/ In general, dataclasses eliminate so much boilerplate code that I use them wherever I can. |
mwhittaker
left a comment
mwhittaker
left a comment
There was a problem hiding this comment.
Thanks for the PR! I spent some time reading up on dataclasses, and I think the switch here may not be worth it.
- I don't think we benefit much since there's not a lot of boilerplate to remove in this case.
- The dataclass also introduces
capacityintoNodewhich it didn't previously have, and it's not clear whatcapacitymeans. For example, if we construct a node likeNode('a', read_capacity=100, write_capacity=200), there's not a nice value forcapacity. - Also since
Nodeis part of theExprhierarchy, it feels weird to me thatNodewould be a dataclass but notAndorOr.
| write_capacity: Optional[float] = None | ||
| latency: Optional[datetime.timedelta] = None | ||
|
|
||
| def post_init(self): |
There was a problem hiding this comment.
This should be __post_init__.
Dataclasses make the code more compact and generate many convenience functions.
|
Yes - it saves only 10 lines in this diff. Not an earth shattering difference :) I've addressed the bugs you found in the code and simplified the typing and default values. I'll leave it here just in case you end up using dataclasses elsewhere and then want to convert this one for consistency. |
2 participants
Dataclasses make the code more compact and generate many convenience
functions.