Rework cross section #596
Merged
Rework cross section #596
Conversation
This file contains 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
Reviewer's Guide by SourceryThis pull request introduces a new Class diagram for the new CrossSection hierarchyclassDiagram
class TCrossSection~TUnit~ {
<<abstract>>
-_base_cross_section: SymmetricalCrossSection
+width(): TUnit
+layer(): LayerInfo
+enclosure(): LayerEnclosure
+sections(): dict
+radius(): TUnit
+radius_min(): TUnit
}
class SymmetricalCrossSection {
+width: int
+enclosure: LayerEnclosure
+name: str
+radius: int
+radius_min: int
+main_layer(): LayerInfo
}
class CrossSection {
+__init__(width: int, layer: LayerInfo, sections: list)
+sections(): tuple
}
class DCrossSection {
+__init__(width: int, layer: LayerInfo, sections: list)
}
TCrossSection <|-- CrossSection
TCrossSection <|-- DCrossSection
CrossSection *-- SymmetricalCrossSection : uses
DCrossSection *-- SymmetricalCrossSection : uses
note for TCrossSection "Generic abstract base class for cross sections"
note for SymmetricalCrossSection "Base implementation for cross sections"
note for CrossSection "Concrete implementation for integer units"
note for DCrossSection "Concrete implementation for database units"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
0565fc4 to
5ec2a49
Compare
f175fce to
4474702
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #596 +/- ##
==========================================
- Coverage 65.97% 65.88% -0.09%
==========================================
Files 64 64
Lines 9516 9636 +120
Branches 1772 1786 +14
==========================================
+ Hits 6278 6349 +71
- Misses 2717 2763 +46
- Partials 521 524 +3 ☔ View full report in Codecov by Sentry. |
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.
| if not bbox_offsets: | ||
| bbox_offsets = [0 for _ in range(len(bbox_layers))] | ||
| else: | ||
| if not len(bbox_offsets) == len(bbox_layers): |
There was a problem hiding this comment.
suggestion (code-quality): Simplify logical expression using De Morgan identities (de-morgan)
Suggested change
| if not len(bbox_offsets) == len(bbox_layers): | |
| if len(bbox_offsets) != len(bbox_layers): |
| if not bbox_offsets: | ||
| bbox_offsets = [0 for _ in range(len(bbox_layers))] | ||
| else: | ||
| if not len(bbox_offsets) == len(bbox_layers): |
There was a problem hiding this comment.
suggestion (code-quality): Simplify logical expression using De Morgan identities (de-morgan)
Suggested change
| if not len(bbox_offsets) == len(bbox_layers): | |
| if len(bbox_offsets) != len(bbox_layers): |
| if cross_section.name not in self.cross_sections: | ||
| self.cross_sections[cross_section.name] = cross_section | ||
| return cross_section | ||
| return self.cross_sections[cross_section.name] | ||
| xs = self.cross_sections[cross_section.name] | ||
| if not xs == cross_section: |
There was a problem hiding this comment.
suggestion (code-quality): Simplify logical expression using De Morgan identities (de-morgan)
Suggested change
| if not xs == cross_section: | |
| if xs != cross_section: |
| @@ -540,6 +537,7 @@ | |||
| layer_sections: dict[kdb.LayerInfo, LayerSection] | |||
| _name: str | None = PrivateAttr() | |||
| main_layer: kdb.LayerInfo | None | |||
| bbox_sections: dict[kdb.LayerInfo, tuple[int, int, int, int]] | |||
|
|
|||
| def __init__( | |||
There was a problem hiding this comment.
issue (code-quality): Replace mutable default arguments with None (default-mutable-arg)
Comment on lines
+428
to
430
| if other is None: | ||
| return None | ||
| return kdb.CplxTrans(self.layout.dbu) * other |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| if other is None: | |
| return None | |
| return kdb.CplxTrans(self.layout.dbu) * other | |
| return None if other is None else kdb.CplxTrans(self.layout.dbu) * other |
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.
Summary by Sourcery
Refactor the cross section implementation to use a more generic approach based on abstract base classes and generics. Rename width parameter in extrude_path and extrude_path_points functions from µm to um.
New Features:
TCrossSectionabstract base class for defining cross sections with different unit types.Tests: