Skip to content

Conversation

@marcorudolphflex
Copy link
Contributor

@marcorudolphflex marcorudolphflex commented Nov 4, 2025

…brary-and-geometry

follow up of #2921

Here, all functions of the packages tidy3d/config, tidy3d/material_library and tidy3d/components/geometry were extended with typedefs for mypy.

Greptile Overview

Updated On: 2025-11-04 11:34:44 UTC

Greptile Summary

This PR adds comprehensive type annotations to three packages (tidy3d/config, tidy3d/material_library, and tidy3d/components/geometry) as a follow-up to PR #2921. The changes enable mypy type checking by adding these packages to the mypy configuration in pyproject.toml.

Key changes:

  • Added return type annotations to properties, methods, and functions across all three packages
  • Imported necessary types (LogLevel, NDArray, Cell, Trimesh, etc.) and used TYPE_CHECKING guards for forward references
  • Added typing.Self for methods returning the same type
  • Properly annotated complex types like NDArray[float] and Union types

Issues found:

  • In tidy3d/components/geometry/base.py, the __or__ method returns GeometryGroup but is annotated as returning ClipOperation

Confidence Score: 4/5

  • This PR is mostly safe to merge with one type annotation fix needed
  • The PR adds comprehensive type annotations across three packages, which improves code quality and enables static type checking. However, there is one incorrect return type annotation in the __or__ method that needs to be corrected. The change is otherwise straightforward and low-risk, consisting primarily of adding type hints without changing logic.
  • tidy3d/components/geometry/base.py requires attention for the incorrect return type annotation on the __or__ method

Important Files Changed

File Analysis

Filename Score Overview
pyproject.toml 5/5 Added new packages to mypy file list for type checking
tidy3d/material_library/material_library.py 5/5 Added type annotations and imported necessary types for IPython and rich
tidy3d/components/geometry/base.py 3/5 Added extensive type annotations to methods; incorrect return type on __or__ method (returns GeometryGroup but annotated as ClipOperation)
tidy3d/components/geometry/polyslab.py 5/5 Added comprehensive type annotations to methods and functions

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant MyPy as MyPy Type Checker
    participant Config as tidy3d/config
    participant MatLib as tidy3d/material_library
    participant Geom as tidy3d/components/geometry
    
    Dev->>MyPy: Run mypy type checking
    MyPy->>Config: Check type annotations
    Config-->>MyPy: ✓ All types valid
    MyPy->>MatLib: Check type annotations
    MatLib-->>MyPy: ✓ All types valid
    MyPy->>Geom: Check type annotations
    Geom-->>MyPy: ⚠️ __or__ return type mismatch
    MyPy->>Dev: Report type errors
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@marcorudolphflex marcorudolphflex force-pushed the FXC-3958-mypy-implement-type-defs-in-config-material-library-and-geometry branch 3 times, most recently from 83bb24e to 47fb565 Compare November 4, 2025 12:04
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

Diff Coverage

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

  • tidy3d/components/geometry/base.py (93.8%): Missing lines 1485,1494,1518
  • tidy3d/components/geometry/mesh.py (100%)
  • tidy3d/components/geometry/polyslab.py (100%)
  • tidy3d/components/geometry/primitives.py (100%)
  • tidy3d/components/geometry/utils.py (100%)
  • tidy3d/components/geometry/utils_2d.py (100%)
  • tidy3d/config/legacy.py (100%)
  • tidy3d/config/manager.py (100%)
  • tidy3d/material_library/material_library.py (100%)
  • tidy3d/material_library/util.py (100%)

Summary

  • Total: 145 lines
  • Missing: 3 lines
  • Coverage: 97%

tidy3d/components/geometry/base.py

Lines 1481-1489

  1481         # This allows the user to write sum(geometries...) with the default start=0
  1482         if isinstance(other, int):
  1483             return self
  1484         if not isinstance(other, Geometry):
! 1485             return NotImplemented  # type: ignore[return-value]
  1486         return GeometryGroup(geometries=self._as_union() + other._as_union())
  1487 
  1488     def __radd__(self, other: Union[int, Geometry]) -> Union[Self, GeometryGroup]:
  1489         """Union of geometries"""

Lines 1490-1498

  1490         # This allows the user to write sum(geometries...) with the default start=0
  1491         if isinstance(other, int):
  1492             return self
  1493         if not isinstance(other, Geometry):
! 1494             return NotImplemented  # type: ignore[return-value]
  1495         return GeometryGroup(geometries=other._as_union() + self._as_union())
  1496 
  1497     def __or__(self, other: Geometry) -> GeometryGroup:
  1498         """Union of geometries"""

Lines 1514-1522

  1514 
  1515     def __sub__(self, other: Geometry) -> ClipOperation:
  1516         """Difference of geometries"""
  1517         if not isinstance(other, Geometry):
! 1518             return NotImplemented  # type: ignore[return-value]
  1519         return ClipOperation(operation="difference", geometry_a=self, geometry_b=other)
  1520 
  1521     def __xor__(self, other: Geometry) -> ClipOperation:
  1522         """Symmetric difference of geometries"""

@marcorudolphflex marcorudolphflex force-pushed the FXC-3958-mypy-implement-type-defs-in-config-material-library-and-geometry branch from 47fb565 to bb013d1 Compare November 4, 2025 14:33
Copy link
Collaborator

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

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

LGTM!

@yaugenst-flex yaugenst-flex added this pull request to the merge queue Nov 5, 2025
Merged via the queue into develop with commit 9ad91f1 Nov 5, 2025
37 checks passed
@yaugenst-flex yaugenst-flex deleted the FXC-3958-mypy-implement-type-defs-in-config-material-library-and-geometry branch November 5, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants