Skip to content

Dev 9850 remove list boilerplate - #240

Draft
Boris-Filin wants to merge 22 commits into
mainfrom
DEV-9850-remove-list-boilerplate
Draft

Dev 9850 remove list boilerplate#240
Boris-Filin wants to merge 22 commits into
mainfrom
DEV-9850-remove-list-boilerplate

Conversation

@Boris-Filin

@Boris-Filin Boris-Filin commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Add a new suite of classes such as MridList that 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

  • I have performed a self review of my own code (including checking issues raised when creating the PR).
  • I have added/updated unit tests for these changes, and if not I have explained why they are not necessary.
  • I have commented my code in any hard-to-understand or hacky areas.
  • I have handled all new warnings generated by the compiler or IDE.
  • I have rebased onto the target branch (usually main).

Security

When developing applications, use following guidelines for information security considerations:

  • Access to applications should be protected with security keys/tokens or usernames and passwords;
  • All sessions are encrypted if possible;
  • All application input is sanitised before being acted on (ie SQL statements, etc);
  • Log messages, and especially client-facing ones, must be handled securely and must not leak credentials information (internal URLs, passwords, tokens).
  • I have considered if this change impacts information security and made sure those impacts are handled.

Documentation

  • I have updated the changelog.
  • I have updated any documentation required for these changes.

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.

@ryanjcoleman

Copy link
Copy Markdown
Member

Task linked: DEV-9850 Update Python SDK

Boris Filin added 22 commits August 3, 2026 15:56
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>
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>
Signed-off-by: Boris Filin <boris.filin@zepben.com>
@charlta
charlta force-pushed the DEV-9850-remove-list-boilerplate branch from c78d3ec to d069c21 Compare August 3, 2026 06:02

@charlta charlta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)}),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Should this be an IndexError?

from zepben.ewb.boilerplate.collections.abstract_backed_collection import AbstractBackedCollection


class HasMrid(Protocol):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants