-
Notifications
You must be signed in to change notification settings - Fork 63
XY transposable plots V3 part 1: Geometry and Structure (issue #1072) #2687
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
"""Tests objects shared by multiple components.""" | ||
|
||
from __future__ import annotations | ||
|
||
import random | ||
|
||
import pytest | ||
from shapely.geometry import Point | ||
|
||
from tidy3d.components.utils import pop_axis_and_swap, shape_swap_xy, unpop_axis_and_swap | ||
|
||
|
||
@pytest.mark.parametrize("transpose", [True, False]) | ||
def test_pop_axis_and_swap(transpose): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should also check if the swapping is done properly |
||
for axis in range(3): | ||
coords = (1, 2, 3) | ||
Lz, (Lx, Ly) = pop_axis_and_swap(coords, axis=axis, transpose=transpose) | ||
_coords = unpop_axis_and_swap(Lz, (Lx, Ly), axis=axis, transpose=transpose) | ||
assert all(c == _c for (c, _c) in zip(coords, _coords)) | ||
_Lz, (_Lx, _Ly) = pop_axis_and_swap(_coords, axis=axis, transpose=transpose) | ||
assert Lz == _Lz | ||
assert Lx == _Lx | ||
assert Ly == _Ly | ||
|
||
|
||
def test_shape_swap_xy(): | ||
p_orig = Point(random.random(), random.random()) | ||
p_new = shape_swap_xy(p_orig) | ||
assert (p_new.coords[0][0], p_new.coords[0][1]) == (p_orig.coords[0][1], p_orig.coords[0][0]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
SpatialDataArray, | ||
) | ||
from tidy3d.components.types import ArrayLike, Ax, Axis, Bound | ||
from tidy3d.components.utils import pop_axis_and_swap | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: The modifications to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those subsequent changes are currently bundled into a single PR (PR2544), which is based on top of this PR. Regarding this PR:I verified this function is working by running the |
||
from tidy3d.components.viz import add_ax_if_none, equal_aspect, plot_params_grid | ||
from tidy3d.constants import inf | ||
from tidy3d.exceptions import DataError | ||
|
@@ -570,9 +571,10 @@ def does_cover(self, bounds: Bound) -> bool: | |
|
||
""" Plotting """ | ||
|
||
@property | ||
def _triangulation_obj(self) -> Triangulation: | ||
def _triangulation_obj(self, transpose: bool = False) -> Triangulation: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Converting this from a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree, should be added back There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry. That was bad. I should have defined a new function here. |
||
"""Matplotlib triangular representation of the grid to use in plotting.""" | ||
if transpose: | ||
return Triangulation(self.points[:, 1], self.points[:, 0], self.cells) | ||
return Triangulation(self.points[:, 0], self.points[:, 1], self.cells) | ||
|
||
@equal_aspect | ||
|
@@ -589,6 +591,7 @@ def plot( | |
shading: Literal["gourand", "flat"] = "gouraud", | ||
cbar_kwargs: Optional[dict] = None, | ||
pcolor_kwargs: Optional[dict] = None, | ||
transpose: bool = False, | ||
) -> Ax: | ||
"""Plot the data field and/or the unstructured grid. | ||
|
||
|
@@ -616,6 +619,8 @@ def plot( | |
Additional parameters passed to colorbar object. | ||
pcolor_kwargs: Dict = {} | ||
Additional parameters passed to ax.tripcolor() | ||
transpose : bool = False | ||
Swap horizontal and vertical axes. (This overrides the default ascending axis order) | ||
|
||
Returns | ||
------- | ||
|
@@ -639,7 +644,7 @@ def plot( | |
f"{self._values_coords_dict} before plotting." | ||
) | ||
plot_obj = ax.tripcolor( | ||
self._triangulation_obj, | ||
self._triangulation_obj(transpose=transpose), | ||
self.values.data.ravel(), | ||
shading=shading, | ||
cmap=cmap, | ||
|
@@ -657,14 +662,15 @@ def plot( | |
# plot grid if requested | ||
if grid: | ||
ax.triplot( | ||
self._triangulation_obj, | ||
self._triangulation_obj(transpose=transpose), | ||
color=plot_params_grid.edgecolor, | ||
linewidth=plot_params_grid.linewidth, | ||
) | ||
|
||
# set labels and titles | ||
ax_labels = ["x", "y", "z"] | ||
normal_axis_name = ax_labels.pop(self.normal_axis) | ||
normal_axis_name, ax_labels = pop_axis_and_swap( | ||
"xyz", self.normal_axis, transpose=transpose | ||
) | ||
ax.set_xlabel(ax_labels[0]) | ||
ax.set_ylabel(ax_labels[1]) | ||
ax.set_title(f"{normal_axis_name} = {self.normal_pos}") | ||
|
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.
logic: The parametrize decorator uses
zip(GEO_TYPES, [True, False])
which will only test each geometry type with one transpose value. This should beproduct(GEO_TYPES, [True, False])
to test all combinations.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'd suggest simply doubling up on
pytest.mark.parameterize
eg.