New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pdk improvements #2694
Pdk improvements #2694
Conversation
joamatab
commented
Apr 22, 2024
•
edited
edited
- better defaults args for generic pdk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @joamatab - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
input_straight_cross_section: CrossSectionSpec | None = None, | ||
output_straight_cross_section: CrossSectionSpec | None = None, | ||
cross_section: CrossSectionSpec = "xs_sc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_refinement): Consider initializing input_straight_cross_section
and output_straight_cross_section
directly to cross_section
if not provided.
This would simplify the logic and reduce redundancy in the code.
input_straight_cross_section: CrossSectionSpec | None = None, | |
output_straight_cross_section: CrossSectionSpec | None = None, | |
cross_section: CrossSectionSpec = "xs_sc", | |
input_straight_cross_section: CrossSectionSpec | None = None, | |
output_straight_cross_section: CrossSectionSpec | None = None, | |
cross_section: CrossSectionSpec = input_straight_cross_section or output_straight_cross_section or "xs_sc", |
@@ -7,7 +7,7 @@ | |||
|
|||
@gf.cell | |||
def spiral_double( | |||
min_bend_radius: float = 10.0, | |||
min_bend_radius: float | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_clarification): Clarify in the documentation the behavior when min_bend_radius
is None
.
xs = gf.get_cross_section(cross_section) | ||
min_bend_radius = min_bend_radius or xs.radius |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Validate xs.radius
is not None
before assignment to avoid potential runtime errors.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2694 +/- ##
==========================================
+ Coverage 71.71% 71.73% +0.01%
==========================================
Files 366 366
Lines 23769 23772 +3
Branches 3876 3875 -1
==========================================
+ Hits 17047 17052 +5
+ Misses 5592 5591 -1
+ Partials 1130 1129 -1 ☔ View full report in Codecov by Sentry. |