Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved naming regressions allow duplicate block declarations and constraint names, while export names are malformed and source locators report incorrect lines.
4 open issues
What changed in this PR
Refactors element naming across the core and electronics libraries, separating namespace management from element storage and using deferred names to initialize parameters at declaration time.
Changes:
- Introduces
NamespaceManager,NamingContainer, deferred names, and naming-error diagnostics. - Migrates element collections to lists and updates naming, serialization, and parameter initialization.
- Adds naming-error tests and refreshes example net ordering.
| File | Description |
|---|---|
| examples/PcbBot/PcbBot.svgpcb.js | Updates generated net ordering. |
| examples/PcbBot/PcbBot.net.ref | Aligns reference net ordering and codes. |
| examples/Multimeter/Multimeter.svgpcb.js | Updates generated net ordering. |
| examples/Multimeter/Multimeter.net.ref | Aligns reference net ordering and codes. |
| edg/electronics_model/SubboardBlock.py | Migrates external-block and export-tap naming. |
| edg/electronics_model/PinAssignmentUtil.py | Updates bundled-port traversal. |
| edg/electronics_model/PassivePort.py | Updates adapter parameter lookup. |
| edg/electronics_model/CircuitBlock.py | Uses container-based footprint pin paths. |
| edg/electronics_interfaces/CircuitPackingBlock.py | Updates packing metadata naming. |
| edg/core/Util.py | Adapts diagnostic dictionaries to list-backed collections. |
| edg/core/test_block_errors.py | Adds missing- and duplicate-name tests. |
| edg/core/Ports.py | Migrates port storage, cloning, and naming. |
| edg/core/MultipackBlock.py | Updates packed-element registration. |
| edg/core/Link.py | Updates connection naming and parameter initialization handling. |
| edg/core/HierarchyBlock.py | Migrates hierarchy serialization and unifies empty-chain handling. |
| edg/core/HdlUserExceptions.py | Adds ElementNamingError. |
| edg/core/Generator.py | Updates generator error parameter names. |
| edg/core/DesignTop.py | Migrates packing references to new naming APIs. |
| edg/core/Core.py | Introduces namespace management and the naming mixin. |
| edg/core/Blocks.py | Uses list-backed elements and declaration-time parameter initialization. |
| edg/core/BlockInterfaceMixin.py | Updates the base-type bound to LibraryElement. |
| edg/core/Array.py | Simplifies vector element-name lookup. |
| edg/core/__init__.py | Removes the SubElementDict export. |
| edg/abstract_parts/PinMappable.py | Updates resource subport traversal. |
| edg/abstract_parts/IoControllerWrapper.py | Migrates recursive port naming. |
| edg/abstract_parts/IoController.py | Updates transformed-port and allocation naming. |
| edg/abstract_parts/Connectors.py | Renames the test-point argument to tp_name. |
Review effort: Balanced
Comment on lines
+442
to
446
| for i, constraint in enumerate(self._constraints): | ||
| name = self._namer.name(constraint) | ||
| if name is None: | ||
| name = f"anon_constr_{i}" | ||
| constraint._populate_expr_proto(edgir.add_pair(pb.constraints, name), ref_map) |
Comment on lines
+67
to
+68
| else: | ||
| pass # has valid name |
| for i in range(self._depth): | ||
| frame = frame.f_back | ||
| assert frame is not None | ||
| return f"{frame.f_code.co_filename}:{frame.f_lineno}" |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unifies the subelement dict and manager structure into one namespace manager with one clear responsibility. The old subelement dicts are now managed as lists by their container, which is orthogonal to the naming.
Separates naming-capable objects into a NamingContainer mixin.
Adds capability for dependent names, where a name can be defined with a lambda that is resolved later.
The new naming APIs are consistently structured as base.name_of(inner)
Other changes:
Opportunistic refactors:
Future PRs:
Contributor License Agreement
By submitting this pull request, I agree to dual-license this contribution under both BSD 3-clause and Apache License, Version 2.0.