XY transposable plots V3 part 1: Geometry and Structure (issue #1072) #2687

jewettaijfc · 2025-07-23T06:58:50Z

This PR adds the transpose argument to all plots for Geometry objects and Structure objects.
This PR is part of a series of PRs related to issue Add option to transpose plot #1072 . (The next PR will enable Scene.plot().)

Test instructions

To test this PR, follow the instructions in this file (after unzipping):
tidy3d_issue1072_v3_part1_geom.ipynb.zip

Notes

This PR is part of a series of PRs which overhaul PR2544. That PR was far too big and messy to review easily.
The easiest way to split up that huge PR was by files. (Different files in different PRs.)
Consequently, this PR includes a few code changes that are not strictly needed for plotting Geometry and Structure objects. However they will soon be needed in subsequent PRs in the coming days.

Greptile Summary

This PR implements the transpose parameter for geometry and structure plotting methods as part of a larger effort to add XY transposable plots functionality (issue #1072). The changes allow users to swap horizontal and vertical axes in 2D plots by setting transpose=True, overriding the default ascending axis order.

The implementation introduces several key components:

New utility functions in tidy3d/components/utils.py:
- pop_axis_and_swap() and unpop_axis_and_swap() extend existing geometry axis manipulation with transpose capability
- shape_swap_xy() handles coordinate swapping for Shapely objects using affine transformations
- warn_untested_argument() provides standardized warnings for untested argument combinations
Core geometry plotting updates in tidy3d/components/geometry/base.py:
- All plotting methods now accept the transpose parameter (defaulting to False)
- Systematic replacement of self.pop_axis() calls with pop_axis_and_swap() for coordinate handling
- Special handling for Point geometries with infinite coordinates that can't use standard Shapely transformations
Extended support across geometry types:
- TriangleMesh, PolySlab, and other geometry classes updated to pass through the transpose parameter
- TriangularGridDataset plotting modified to swap triangulation coordinates when transposed
- Geometry utility functions updated to support transpose in plane intersection calculations
Structure plotting integration:
- AbstractStructure.plot() method updated to accept and forward the transpose parameter to underlying geometry plotting
Comprehensive test coverage:
- New tests for utility functions ensuring bidirectional consistency
- Parametrized testing across all geometry types with both transpose values
- Updated existing geometry tests to cover transpose functionality

The changes maintain full backward compatibility and follow established patterns in the codebase. The implementation is systematic and ensures consistent behavior across all geometry and structure plotting methods.

Confidence score: 4/5

• This PR introduces well-structured functionality with comprehensive test coverage, but has a few minor issues that should be addressed.
• The score reflects one potential test reproducibility issue with random coordinate generation and a minor typo in documentation, but the core transpose functionality appears solid and well-implemented.
• Files needing more attention: tests/test_components/test_utils.py (random test data), tidy3d/components/utils.py (docstring typo)

jewettaijfc · 2025-07-23T07:06:28Z

tidy3d/components/data/unstructured/triangular.py

@@ -23,6 +23,7 @@
    SpatialDataArray,
 )
 from tidy3d.components.types import ArrayLike, Ax, Axis, Bound
+from tidy3d.components.utils import pop_axis_and_swap


Note: The modifications to the triangular.py were probably not necessary for Geometry or Structure plotting. But I'd like to include these changes in this PR anyway. The ability to plot triangle meshes is needed by several other PRs which are coming soon, so I want to land them in this PR first.

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 plot_mesh() function in the ChargeSolver.ipynb example with transpose=True, and verifying that the horizontal and vertical axes were swapped. However you will need the other changes in PR2544 for that particular example to work, because it does not invoke this function directly. Perhaps the changes to this file should be moved into that PR, since the other code modified in this PR does not depend on this file.

jewettaijfc · 2025-07-23T07:12:53Z

tidy3d/components/geometry/base.py

-            ax.scatter(shape.x, shape.y, color=plot_params.facecolor)
+            xcrds, ycrds = shape.x, shape.y
+            if transpose:  # shape.x and shape.y might be infinite, so shape_swap_xy(shape) won't
+                xcrds, ycrds = ycrds, xcrds  # work.  Instead we must swap coordinates manually.


If you're curious why shape_swap_xy(shape) doesn't work when the coordinates are infinite, it's because it multiplies the coordinates by a matrix that swaps the X and Y axes. (The shapely library has a function to apply transformations to coordinates. I used it to implement shape_swap_xy().) Unfortunately, if any of the coordinates are inf, the resulting coordinates are nan after matrix multiplication.

jewettaijfc · 2025-07-23T07:36:23Z

tidy3d/components/utils.py

+    log.warning(
+        f"UNTESTED!  The `{prefix}{func_name}()` function has not yet been tested with `{arg}={val}`.",
+        log_once=True,
+    )


The warn_untested_argument() function is not used in this PR. I am not sure if I will need it. If not (after I've submitted all the PRs in this series), I will delete this function. But let's keep it here for now. (I need it to debug the other PRs in the series.)

greptile-apps

_{12 files reviewed, 3 comments}

_{Edit Code Review Bot Settings | Greptile}

greptile-apps · 2025-07-23T07:38:55Z

tidy3d/components/utils.py

+    transpose: bool = False,
+) -> tuple[Any, Any, Any]:
+    """
+    ``unpop_axis_and_swap()`` is identical to ``Geompetry.unpop_axis()``, except that


syntax: Typo: 'Geompetry' should be 'Geometry'

Suggested change

``unpop_axis_and_swap()`` is identical to ``Geompetry.unpop_axis()``, except that

``unpop_axis_and_swap()`` is identical to ``Geometry.unpop_axis()``, except that

greptile-apps · 2025-07-23T07:39:03Z

tidy3d/components/data/unstructured/triangular.py

@@ -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:


logic: Converting this from a @property to a method breaks backward compatibility for any code that accessed _triangulation_obj as a property

agree, should be added back

Sorry. That was bad. I should have defined a new function here.

greptile-apps · 2025-07-23T07:43:21Z

tests/test_components/test_geometry.py

-@pytest.mark.parametrize("component", GEO_TYPES)
-def test_plot(component):
-    _ = component.plot(z=0, ax=AX)
+@pytest.mark.parametrize("component, transpose", zip(GEO_TYPES, [True, False]))


logic: The parametrize decorator uses zip(GEO_TYPES, [True, False]) which will only test each geometry type with one transpose value. This should be product(GEO_TYPES, [True, False]) to test all combinations.

Suggested change

@pytest.mark.parametrize("component, transpose", zip(GEO_TYPES, [True, False]))

@pytest.mark.parametrize("component, transpose", list(itertools.product(GEO_TYPES, [True, False])))

I'd suggest simply doubling up on pytest.mark.parameterize eg.

@pytest.mark.parameterize("component" , ...) @pytest.mark.parameterize("transpose" , ...)

github-actions · 2025-07-23T07:52:31Z

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

tidy3d/components/data/unstructured/triangular.py (80.0%): Missing lines 577
tidy3d/components/geometry/base.py (96.0%): Missing lines 595
tidy3d/components/geometry/mesh.py (100%)
tidy3d/components/geometry/polyslab.py (100%)
tidy3d/components/geometry/primitives.py (100%)
tidy3d/components/geometry/utils.py (66.7%): Missing lines 80
tidy3d/components/structure.py (100%)
tidy3d/components/utils.py (84.6%): Missing lines 100-103

Summary

Total: 67 lines
Missing: 7 lines
Coverage: 89%

tidy3d/components/data/unstructured/triangular.py

  573 
  574     def _triangulation_obj(self, transpose: bool = False) -> Triangulation:
  575         """Matplotlib triangular representation of the grid to use in plotting."""
  576         if transpose:
! 577             return Triangulation(self.points[:, 1], self.points[:, 0], self.cells)
  578         return Triangulation(self.points[:, 0], self.points[:, 1], self.cells)
  579 
  580     @equal_aspect
  581     @add_ax_if_none

tidy3d/components/geometry/base.py

  591             ax.plot(xs, ys, color=plot_params.facecolor, linewidth=plot_params.linewidth)
  592         elif _shape.geom_type == "Point":
  593             xcrds, ycrds = shape.x, shape.y
  594             if transpose:  # shape.x and shape.y might be infinite, so shape_swap_xy(shape) won't
! 595                 xcrds, ycrds = ycrds, xcrds  # work.  Instead we must swap coordinates manually.
  596             ax.scatter(xcrds, ycrds, color=plot_params.facecolor)
  597         else:
  598             patch = polygon_patch(_shape, **plot_params.to_kwargs())
  599             ax.add_artist(patch)

tidy3d/components/geometry/utils.py

  76 
  77         # Append each of them and their property information to the list of shapes
  78         for shape in shapes_plane:
  79             if transpose:
! 80                 shape = shape_swap_xy(shape)
  81             shapes.append((prop, shape, shape.bounds))
  82 
  83     background_shapes = []
  84     for prop, shape, bounds in shapes:

tidy3d/components/utils.py

   96 
   97 def warn_untested_argument(cls_name: Optional[str], func_name: str, arg: str, val: str):
   98     """Generic warning message if a function has never been manually tested with ``arg=val``. (This"
   99     is typically used for plot functions where manual tests and visual confirmation is needed.)"""
! 100     prefix = ""
! 101     if cls_name:
! 102         prefix = cls_name + "."
! 103     log.warning(
  104         f"UNTESTED!  The `{prefix}{func_name}()` function has not yet been tested with `{arg}={val}`.",
  105         log_once=True,
  106     )

tests/test_components/test_geometry.py

…t which swaps the horizontal and vertical axes"

tylerflex · 2025-07-25T19:56:25Z

tests/test_components/test_utils.py

+
+
+@pytest.mark.parametrize("transpose", [True, False])
+def test_pop_axis_and_swap(transpose):


should also check if the swapping is done properly

tylerflex · 2025-07-25T19:57:33Z

tidy3d/components/geometry/base.py

@@ -263,15 +264,19 @@ def intersections_plane(
        origin = self.unpop_axis(position, (0, 0), axis=axis)
        normal = self.unpop_axis(1, (0, 0), axis=axis)
        to_2D = np.eye(4)
-        if axis != 2:


need to understand why this was changed

Hi Tyler. Please revert this change back to the way it was.

If you're curious, I removed axis != 2 during a time when I was making big modifications to all of the intersections_...() functions in Tidy3D (enabling them to support transpose=True). The axis != 2 conditional got in the way of making that change.

Later, I figured out a way to avoid making any changes to any of the intersections_...() functions. (This was a big simplification.) I tried to revert all of the older changes I made to these functions, but I guess I forgot to undo this change. Sorry about that. Please change it back.

(If I remember correctly, the next two lines have no effect if axis==2. That's why Lucas added the if axis != 2 there when he wrote the original code. Before I made this change, I discussed it with him over slack to confirm I could safely remove the if axis != 2, and he said yes. Either way, feel free to change it back.)

tylerflex · 2025-07-25T19:58:41Z

tidy3d/components/geometry/base.py

@@ -1567,7 +1610,6 @@ def intersections_tilted_plane(
            For more details refer to
            `Shapely's Documentation <https://shapely.readthedocs.io/en/stable/project.html>`_.
        """
-


will remove newline changes

tylerflex · 2025-07-25T19:59:18Z

tidy3d/components/utils.py

+from tidy3d.log import log
+
+
+def pop_axis_and_swap(


would rather this live in the same place as pop_axis_and_swap

You mean pop_axis()?
That was my plan originally. pop_axis_and_swap() was originally defined in components/geometry/base.py (immediately after pop_axis()). (And it was originally a wrapper function that extended the functionality of pop_axis().)

But I discovered later that there are plotting functions in plugins/ which also need to support transpose=True. (Example here.) That's why I moved those functions out of the components/geometry/ folder (paving the way for changes introduced later in PR2544).

Either way, I agree that it's better for them to be defined in the same file, and better if pop_axis_and_swap() wraps/extends pop_axis(). Feel free to move these functions back (or alternatively, move the pop_axis() and unpop_axis() functions here).

tylerflex · 2025-07-25T20:04:46Z

tidy3d/components/geometry/utils_2d.py

no changes needed

tylerflex · 2025-07-25T20:05:48Z

tidy3d/components/utils.py

+from tidy3d.log import log
+
+
+def pop_axis_and_swap(


these functions should wrap pop_axis and unpop_axis to reuse code

See comment above.

tylerflex · 2025-07-25T20:06:16Z

tidy3d/components/utils.py

+    return tuple(coords)
+
+
+def shape_swap_xy(shape: Shapely) -> Shapely:


this can go into Geometry too

tylerflex · 2025-07-25T20:06:26Z

tidy3d/components/utils.py

+    return shape_new
+
+
+def warn_untested_argument(cls_name: Optional[str], func_name: str, arg: str, val: str):


will remove

jewettaijfc requested a review from tylerflex July 23, 2025 06:58

jewettaijfc commented Jul 23, 2025

View reviewed changes

jewettaijfc marked this pull request as ready for review July 23, 2025 07:38

jewettaijfc requested a review from yaugenst-flex July 23, 2025 07:38

greptile-apps bot reviewed Jul 23, 2025

View reviewed changes

jewettaijfc commented Jul 23, 2025

View reviewed changes

tests/test_components/test_geometry.py Show resolved Hide resolved

Geometry.plot() and Structure().plot() accept the "transpose" argumen…

d64d088

…t which swaps the horizontal and vertical axes"

jewettaijfc force-pushed the jewettaijfc/issue1072_v3_part1_geom branch from 4165b47 to d64d088 Compare July 23, 2025 21:58

jewettaijfc mentioned this pull request Jul 23, 2025

XY transposable plots V3 part 2: Updated all remaining plot functions (issue1072) #2544

Open

jewettaijfc changed the title ~~XY transposable plots V3 part1: Geometry and Structure~~ XY transposable plots V3 part 1: Geometry and Structure Jul 23, 2025

jewettaijfc changed the title ~~XY transposable plots V3 part 1: Geometry and Structure~~ XY transposable plots V3 part 1: Geometry and Structure (issue #1072) Jul 23, 2025

tylerflex reviewed Jul 25, 2025

View reviewed changes

tidy3d/components/geometry/utils_2d.py

Copy link

Collaborator

tylerflex Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no changes needed

tylerflex reviewed Jul 25, 2025

View reviewed changes

tidy3d/components/utils.py

return tuple(coords)

def shape_swap_xy(shape: Shapely) -> Shapely:

Copy link

Collaborator

tylerflex Jul 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can go into Geometry too

tylerflex reviewed Jul 25, 2025

View reviewed changes

	``unpop_axis_and_swap()`` is identical to ``Geompetry.unpop_axis()``, except that
	``unpop_axis_and_swap()`` is identical to ``Geometry.unpop_axis()``, except that

	@pytest.mark.parametrize("component, transpose", zip(GEO_TYPES, [True, False]))
	@pytest.mark.parametrize("component, transpose", list(itertools.product(GEO_TYPES, [True, False])))



		@pytest.mark.parametrize("transpose", [True, False])
		def test_pop_axis_and_swap(transpose):

		return tuple(coords)


		def shape_swap_xy(shape: Shapely) -> Shapely:

		return shape_new


		def warn_untested_argument(cls_name: Optional[str], func_name: str, arg: str, val: str):

XY transposable plots V3 part 1: Geometry and Structure (issue #1072) #2687

Are you sure you want to change the base?

XY transposable plots V3 part 1: Geometry and Structure (issue #1072) #2687

Uh oh!

Conversation

jewettaijfc commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test instructions

Notes

Greptile Summary

Confidence score: 4/5

Uh oh!

jewettaijfc Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jewettaij Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Regarding this PR:

Uh oh!

jewettaijfc Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 23, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

Summary

tidy3d/components/data/unstructured/triangular.py

tidy3d/components/geometry/base.py

tidy3d/components/geometry/utils.py

tidy3d/components/utils.py

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jewettaij Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jewettaij Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jewettaij Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jewettaijfc commented Jul 23, 2025 •

edited

Loading

jewettaijfc Jul 23, 2025 •

edited

Loading

jewettaij Jul 28, 2025 •

edited

Loading

jewettaijfc Jul 23, 2025 •

edited

Loading

jewettaij Jul 28, 2025 •

edited

Loading

jewettaij Jul 28, 2025 •

edited

Loading

jewettaij Jul 28, 2025 •

edited

Loading