feat(meshwell): check if mesh order is defined in LayerLevel.info#681
Open
David-GERARD wants to merge 1 commit intogdsfactory:mainfrom
Open
feat(meshwell): check if mesh order is defined in LayerLevel.info#681David-GERARD wants to merge 1 commit intogdsfactory:mainfrom
David-GERARD wants to merge 1 commit intogdsfactory:mainfrom
Conversation
…tead of LayerLevel.mesh_order
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdate meshwell cross-section generation to respect per-layer mesh order defined in LayerLevel.info["mesh_order"] when available, falling back to the LayerLevel.mesh_order default otherwise. Sequence diagram for mesh_order handling in get_meshwell_cross_sectionsequenceDiagram
participant Caller
participant get_meshwell_cross_section
participant layer_level
participant PolySurface
Caller->>get_meshwell_cross_section: call(...)
get_meshwell_cross_section->>layer_level: access info
alt info exists and contains mesh_order
get_meshwell_cross_section->>get_meshwell_cross_section: mesh_order = layer_level.info["mesh_order"]
else info missing or no mesh_order
get_meshwell_cross_section->>layer_level: access mesh_order
get_meshwell_cross_section->>get_meshwell_cross_section: mesh_order = layer_level.mesh_order
end
get_meshwell_cross_section->>PolySurface: instantiate(polygons, physical_name, mesh_order, mesh_bool=True, additive=False)
PolySurface-->>get_meshwell_cross_section: instance
get_meshwell_cross_section-->>Caller: return PolySurface
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider guarding the
layer_level.infoaccess with a type check (e.g., ensuring it's a mapping) or usinggetattr/getto avoid unexpected attribute/type errors ifinfois not a dict-like object. - You can simplify the mesh_order logic by using something like
mesh_order = getattr(getattr(layer_level, 'info', None) or {}, 'get', lambda *a, **k: None)('mesh_order', layer_level.mesh_order)or a small helper function, which keeps the selection logic concise and reusable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider guarding the `layer_level.info` access with a type check (e.g., ensuring it's a mapping) or using `getattr`/`get` to avoid unexpected attribute/type errors if `info` is not a dict-like object.
- You can simplify the mesh_order logic by using something like `mesh_order = getattr(getattr(layer_level, 'info', None) or {}, 'get', lambda *a, **k: None)('mesh_order', layer_level.mesh_order)` or a small helper function, which keeps the selection logic concise and reusable.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
In the cspdk's LayerStack, the mesh order is defined in
LayerLevel.info["mesh_order"], meaning thatLayerLevel.mesh_orderwill default to 3 for every layer.This PR thus adds a check in
gplugins/meshwell/get_meshwell_cross_section.get_meshwell_cross_section()which first verifies ifLayerLevel.info["mesh_order"]exists, if so uses it as the mesh_order, and defaulting toLayerLevel.info["mesh_order"]otherwise.Summary by Sourcery
Bug Fixes: