Add new type ComponentId
#795
Conversation
github-actions
bot
added
part:data-pipeline
There was a problem hiding this comment.
I would really avoid using NewType in this case, also I'm not sure if we should update all component_id to component as how the component is specified now is encoded in the type system, and could open the door to pass a component as an object for example (or even think more on a case-by-case basis if another name is more suitable). But I guess that part is out of scope for this PR.
| from ...timeseries import Fuse | ||
|
|
||
|
|
||
| ComponentId = NewType("ComponentId", int) |
There was a problem hiding this comment.
Why NewType and not dataclass?
NewType allows this for example: ComponentId(1) + 2 and microgrid.battery_pool(priority=ComponentId(1)) which is just weird.
Also the str and repr looks just like a bare int, which might be good or not (I personally would like to at least have repr use ComponentId(1) instead of just 1.
There was a problem hiding this comment.
Honestly, no specific reason, I selected NewType after a quick research, I am not set on using it :)
There was a problem hiding this comment.
In that case I would use a dataclass, I see no benefits in using NewType and a few drawbacks.
There was a problem hiding this comment.
just a data class with one member you mean?
|
I'll try this in the new microgrid API client. |
2 participants
This is a WIP and the current work is 90% done by ChatGPT.
I am currently busy with a few other things, so anyone can take over this PR and finish it.
Otherwise I'll get back to it when my important stuff is done.