Dev 9850 remove list boilerplate - #240
Conversation
|
Task linked: DEV-9850 Update Python SDK |
Signed-off-by: Boris Filin <boris.filin@zepben.com> # Conflicts: # src/zepben/ewb/model/cim/iec61970/base/wires/energy_consumer.py # src/zepben/ewb/model/cim/iec61970/base/wires/energy_source.py # src/zepben/ewb/model/cim/iec61970/base/wires/power_electronics_connection.py # test/dataclass_descriptors/test_dataclass_base.py # test/services/network/tracing/networktrace/test_network_trace_step_path_provider.py # Conflicts: # src/zepben/ewb/model/cim/iec61970/base/core/feeder.py # src/zepben/ewb/model/cim/iec61970/base/core/sub_geographical_region.py # src/zepben/ewb/model/cim/iec61970/base/core/substation.py
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
…onductingequipment Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
…f a generator Signed-off-by: Boris Filin <boris.filin@zepben.com>
…not rely on abstractbackedlist Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com> Signed-off-by: Boris Filin <boris.filin@zepben.com>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
c78d3ec to
d069c21
Compare
charlta
left a comment
There was a problem hiding this comment.
As requested I didn't do a full in depth review, just commenting on things that stood out. Overall looks pretty good.
| def _get_expected(nb_network, line, pt, es, ec, pec, eb, ec_eb1, ec_eb2): | ||
| # -- Bus | ||
| exp_bb0 = (create_terminal_based_id({next(es.terminals), get_term(pt, 1)}), | ||
| exp_bb0 = (create_terminal_based_id({next(iter(es.terminals)), get_term(pt, 1)}), |
There was a problem hiding this comment.
question: Why did iter( need to be added in all these places?
If this is a new requirement it is a major breaking change for anyone using these collections. Can anything be done to remove this requirement?
| size = len(self) | ||
|
|
||
| if not 0 <= index <= size: | ||
| raise ValueError( |
There was a problem hiding this comment.
question: Should this be an IndexError?
| from zepben.ewb.boilerplate.collections.abstract_backed_collection import AbstractBackedCollection | ||
|
|
||
|
|
||
| class HasMrid(Protocol): |
There was a problem hiding this comment.
question: Can this be replaced with Identifiable? If not, what classes stop it?
| The individual phase models for an AcLineSegment. | ||
| `phase` the phase of the required [AcLineSegmentPhase] | ||
| """ | ||
| res = next((it for it in self if it.phase == phase), None) |
There was a problem hiding this comment.
todo: Can we make the way these things check be consistent? The first three in the PR are all different, but essentially doing the same thing of "find first item or throw error".
AcLineSegmentPhaseList:
res = next((it for it in self if it.phase == phase), None)
if res is None:
raise KeyError(phase)
return res
BatteryControlList:
for control in self:
if control.control_mode == control_mode:
return control
raise IndexError(f"No BatteryControl with a control_mode of {control_mode} was found in BatteryUnit {str(self)}")
CurveDataList:
curve_data = next((it for it in self if it.x_value == x), None)
if curve_data:
return curve_data
raise KeyError(x)
|
|
||
| end_terminals: MridCollection[Terminal] = LazyMridList( | ||
| _end_terminals, | ||
| "An Terminal", |
There was a problem hiding this comment.
todo: validate these all have correct language in them. i.e. this should be "A Terminal"
| """Maximum zero sequence Thevenin reactance.""" | ||
|
|
||
| def __init__(self, *args, energy_source_phases: List[EnergySourcePhase] = None, **kwargs): | ||
| def __init__(self, *args, energy_source_phases=None, **kwargs): |
There was a problem hiding this comment.
question: Why are types removed?
| if energy_source_phases: | ||
| for phase in energy_source_phases: | ||
| self.add_phase(phase) | ||
| self.phases.extend(energy_source_phases) |
There was a problem hiding this comment.
question: I assume we have some of these inits left over because parameter names don't match field names. Is this something we can mark as deprecated, as ideally we use the same args as property names.
Description
Add a new suite of classes such as
MridListthat implement nullable list functionality with advanced item validation. This avoids defining list handling methods (len, get, add, remove, clear) for most of the one-to-many relationships in the SDK.Associated tasks
Java parity PR
Test Steps
Run tests (new ones were created to test lists, old ones still run); Load a model from a db and perform arbitrary work on it to ensure no tool functionality was broken
(Note: The change is made to be mostly backwards compatible, this is not expected to be an issue)
Checklist
Code
Security
When developing applications, use following guidelines for information security considerations:
Documentation
Breaking Changes
I have considered if this is a breaking change and will communicate it with other team members by posting it on the Slack breaking-changes channel.
Every CIM class now has to be instantiated with keyword arguments. The only accepted positional argument is mRID.