Improve netlist format and extractor #2846
Conversation
|
@joamatab this PR should be ready to go. it partially resolves #2831 and replaces #2833 for #2831 what still remains is
one thing i notice is that the test_netlists[cross] case unpredictably fails. this appears to be due to problems in serializing the layer. it seems to unpredictably waver between serializing as a layer name vs a layer number (see screenshot below). not sure why this is, but i assume it's a kfactory issue @sebastian-goeldi . just having the tests run in a different order will cause this particular regression test to pass or fail |
|
sorry, actually i should still add one more thing before merging... the capability to parse array-port specifications on the input yaml. i will do that before merging |
|
🧙 Sourcery is reviewing your pull request! Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.
| def _is_array_reference(ref: ComponentReference) -> bool: | ||
| return ref.na > 1 or ref.nb > 1 | ||
|
|
||
|
|
||
| def get_netlist( |
There was a problem hiding this comment.
issue (code-quality): Low code quality found in get_netlist - 7% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
| f"Too many angle brackets (<) in instance specification '{inst_spec}'. Array ref indices should end with <ia.ib>, and otherwise this character should be avoided." | ||
| ) | ||
| if "<" in inst_spec and inst_spec.endswith(">"): | ||
| inst_name, array_spec = inst_spec.split("<") |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Extract code out into function (
extract-method) - Explicitly raise from a previous error [×2] (
raise-from-previous-error)
| instance_dst_name, port_dst_name = ( | ||
| s.strip() for s in dst.split(",") | ||
| ) | ||
| try: |
There was a problem hiding this comment.
issue (code-quality): Explicitly raise from a previous error [×5] (raise-from-previous-error)
|
an additional test (tests/read/test_component_from_yaml.py::test_gds_and_settings[sample_mirror_simple]) broke, related to gdsfactory/kfactory#392, since we now reference the port of the instance directly when connecting, rather than referencing it by name, and this currently gives unexpected behavior when a mirror is applied. otherwise i think this is good to go if you all are happy with it |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2846 +/- ##
==========================================
+ Coverage 68.67% 68.90% +0.22%
==========================================
Files 298 298
Lines 15254 15359 +105
Branches 2229 2253 +24
==========================================
+ Hits 10476 10583 +107
+ Misses 4183 4178 -5
- Partials 595 598 +3 ☔ View full report in Codecov by Sentry. |
…tory into improve-netlist-format
This PR
my_array_ref<ia.ib>,port_name. note that i use a "." between ia and ib rather than a comma so that port parsing everywhere doesn't breakPortconstructor to allowwidthto be an optional attribute (and take the cross-section width if unspecified)Partially resolves #2831