-
Notifications
You must be signed in to change notification settings - Fork 65
chore: rf namespace and consolidation (FXC-3912, FXC-3965) #3017
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
Conversation
698242d to
f7545c2
Compare
f7545c2 to
7cec316
Compare
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.
4 files reviewed, 3 comments
7cec316 to
ecf054d
Compare
dmarek-flex
left a comment
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.
Thanks for doing the dirty work!
ecf054d to
ea1e06d
Compare
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
|
dmarek-flex
left a comment
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.
I think you have got most of the rf-specific stuff
weiliangjin2021
left a comment
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.
A few others:
- medium.py: LossyMetalMedium, SurfaceImpedanceFitterParam, HammerstadSurfaceRoughness, HuraySurfaceRoughness
- grid_spec.py: LayerRefinementSpec, CornerFinderSpec
- subpixel_spec.py: SurfaceImpedance
4a283eb to
17a9897
Compare
17a9897 to
df1daf3
Compare
from tidy3d.rf import *Greptile Overview
Greptile Summary
This PR introduces a new
tidy3d.rfnamespace to consolidate all RF-related classes from the microwave and smatrix plugins, making them accessible viafrom tidy3d.rf import *. The PR also adds backwards compatibility aliases for renamed path integral classes in both the top-level package and microwave plugin.Key changes:
tidy3d/rf.pymodule that exports all RF classes from microwave and smatrix pluginstidy3d/__init__.pyfor four renamed integral classestidy3d/plugins/microwave/__init__.pyfor all five renamed integral classesCHANGELOG.mdto document the new namespace and reorganize breaking changes into "Breaking Changes" and "Planned Deprecation" sectionsCritical issue found: The new
tidy3d.rfmodule is missing theCustomPathIntegral2Dbackwards compatibility alias that exists in the microwave plugin. This breaks backwards compatibility for users importing from the new RF namespace.Confidence Score: 2/5
CustomPathIntegral2D) in the new RF namespace that exists in the microwave plugin, which will break user code that relies on the deprecated class name when importing fromtidy3d.rf. Additionally, the warning message contains emojis which violates the codebase style guidelines.tidy3d/rf.pywhich has critical missing backwards compatibility alias and style violationImportant Files Changed
File Analysis
CustomPathIntegral2Dbackwards compatibility alias and contains emoji in warning message__all__CustomPathIntegral2D, properly exported in__all__tidy3d.rfnamespace and reorganized breaking changes into proper sectionsSequence Diagram
sequenceDiagram participant User participant tidy3d.rf participant tidy3d.__init__ participant microwave_plugin participant components Note over User,components: New RF Namespace Consolidation User->>tidy3d.rf: from tidy3d.rf import * tidy3d.rf->>components: Import path integrals tidy3d.rf->>microwave_plugin: Import models, array_factor, rf_material_library tidy3d.rf->>microwave_plugin: Import smatrix components tidy3d.rf-->>User: Return consolidated RF classes Note over tidy3d.rf: Create backwards compatibility aliases:<br/>CurrentIntegralTypes, VoltageIntegralTypes,<br/>ComponentModeler, CustomPathIntegral2D (MISSING) User->>tidy3d.__init__: from tidy3d import VoltageIntegralAxisAligned tidy3d.__init__->>components: Import AxisAlignedVoltageIntegral Note over tidy3d.__init__: Create backwards compatibility alias:<br/>VoltageIntegralAxisAligned tidy3d.__init__-->>User: Return aliased class User->>microwave_plugin: from tidy3d.plugins.microwave import CustomPathIntegral2D microwave_plugin->>components: Import Custom2DPathIntegral Note over microwave_plugin: Create backwards compatibility alias:<br/>CustomPathIntegral2D microwave_plugin-->>User: Return aliased class