refactor(tidy3d): FXC-4683-move-config-to-new-common-submodule

Author: marcorudolphflex

.github/workflows/tidy3d-python-client-tests.yml

@@ -270,6 +270,27 @@ jobs:
         run: |
           mypy --config-file=pyproject.toml
 
+  ensure-common-imports:
+    name: ensure-common-imports
+    needs: determine-test-scope
+    if: needs.determine-test-scope.outputs.code_quality_tests == 'true'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+          submodules: false
+          persist-credentials: false
+
+      - name: set-python-3.10
+        uses: actions/setup-python@v4
+        with:
+          python-version: '3.10'
+
+      - name: Run tidy3d._common import check
+        run: |
+          python scripts/ensure_imports_from_common.py
+
   zizmor:
     name: Run zizmor 🌈
     runs-on: ubuntu-latest
@@ -983,6 +1004,7 @@ jobs:
       - remote-tests
       - lint
       - mypy
+      - ensure-common-imports
       - verify-schema-change
       - lint-commit-messages
       - lint-branch-name
@@ -1004,6 +1026,12 @@ jobs:
         run: |
           echo "❌ Mypy type checking failed."
           exit 1
+
+      - name: check-common-imports-result
+        if: ${{ needs.determine-test-scope.outputs.code_quality_tests == 'true' && needs.ensure-common-imports.result != 'success' && needs.ensure-common-imports.result != 'skipped' }}
+        run: |
+          echo "❌ tidy3d._common import check failed."
+          exit 1
           
       - name: check-schema-change-verification
         if: ${{ needs.determine-test-scope.outputs.code_quality_tests == 'true' && needs.verify-schema-change.result != 'success' && needs.verify-schema-change.result != 'skipped' }}

scripts/ensure_imports_from_common.py

@@ -0,0 +1,114 @@
+"""Ensure tidy3d._common modules avoid importing from tidy3d outside tidy3d._common."""
+
+from __future__ import annotations
+
+import argparse
+import ast
+import sys
+from collections.abc import Iterable
+from dataclasses import dataclass
+from pathlib import Path
+
+
+@dataclass(frozen=True)
+class ImportViolation:
+    file: str
+    line: int
+    statement: str
+
+
+def parse_args(argv: Iterable[str]) -> argparse.Namespace:
+    parser = argparse.ArgumentParser(
+        description=(
+            "Ensure tidy3d._common does not import from tidy3d modules outside tidy3d._common."
+        )
+    )
+    parser.add_argument(
+        "--root",
+        default="tidy3d/_common",
+        help="Root directory to scan (relative to repo root).",
+    )
+    return parser.parse_args(argv)
+
+
+def main(argv: Iterable[str]) -> None:
+    args = parse_args(argv)
+    repo_root = Path.cwd().resolve()
+    root = (repo_root / args.root).resolve()
+    if not root.exists():
+        print(f"No directory found at {root}. Skipping check.")
+        return
+
+    violations: list[ImportViolation] = []
+    for path in sorted(root.rglob("*.py")):
+        violations.extend(_violations_in_file(path, repo_root))
+
+    if violations:
+        print("Invalid tidy3d imports found in tidy3d._common:")
+        for violation in violations:
+            print(f"{violation.file}:{violation.line}: {violation.statement}")
+        raise SystemExit(1)
+
+    print("No invalid tidy3d imports found in tidy3d._common.")
+
+
+def _violations_in_file(path: Path, repo_root: Path) -> list[ImportViolation]:
+    source = path.read_text(encoding="utf-8")
+    try:
+        tree = ast.parse(source)
+    except SyntaxError as exc:
+        raise SystemExit(f"Syntax error parsing {path}: {exc}") from exc
+
+    rel_path = str(path.relative_to(repo_root))
+    violations: list[ImportViolation] = []
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Import):
+            for alias in node.names:
+                name = alias.name
+                if name == "tidy3d" or (
+                    name.startswith("tidy3d.") and not name.startswith("tidy3d._common")
+                ):
+                    violations.append(
+                        ImportViolation(
+                            file=rel_path,
+                            line=node.lineno,
+                            statement=_statement(source, node),
+                        )
+                    )
+        elif isinstance(node, ast.ImportFrom):
+            if node.level:
+                continue
+            module = node.module
+            if not module:
+                continue
+            if module == "tidy3d":
+                for alias in node.names:
+                    if alias.name != "_common":
+                        violations.append(
+                            ImportViolation(
+                                file=rel_path,
+                                line=node.lineno,
+                                statement=_statement(source, node),
+                            )
+                        )
+                continue
+            if module.startswith("tidy3d.") and not module.startswith("tidy3d._common"):
+                violations.append(
+                    ImportViolation(
+                        file=rel_path,
+                        line=node.lineno,
+                        statement=_statement(source, node),
+                    )
+                )
+    return violations
+
+
+def _statement(source: str, node: ast.AST) -> str:
+    segment = ast.get_source_segment(source, node)
+    if segment:
+        return " ".join(segment.strip().splitlines())
+    return node.__class__.__name__
+
+
+if __name__ == "__main__":
+    main(sys.argv[1:])

tests/config/test_loader.py

@@ -5,8 +5,8 @@
 from click.testing import CliRunner
 from pydantic import Field
 
+from tidy3d._common.config import loader as config_loader  # import from common as it is patched
 from tidy3d.config import get_manager, reload_config
-from tidy3d.config import loader as config_loader
 from tidy3d.config import registry as config_registry
 from tidy3d.config.legacy import finalize_legacy_migration
 from tidy3d.config.loader import migrate_legacy_config

tidy3d/_common/__init__.py

@@ -0,0 +1,12 @@
+"""Runtime environment detection for tidy3d.
+
+This module must have ZERO dependencies on other tidy3d modules to avoid
+circular imports. It is imported very early in the initialization chain.
+"""
+
+from __future__ import annotations
+
+import sys
+
+# Detect WASM/Pyodide environment where web and filesystem features are unavailable
+WASM_BUILD = "pyodide" in sys.modules or sys.platform == "emscripten"

tidy3d/_common/_runtime.py

@@ -0,0 +1,85 @@
+"""Tidy3D configuration system public API."""
+
+from __future__ import annotations
+
+from typing import Any
+
+from .legacy import (
+    LegacyConfigWrapper,
+    LegacyEnvironment,
+    LegacyEnvironmentConfig,
+)
+from .manager import ConfigManager
+from .registry import (
+    get_handlers,
+    get_sections,
+    register_handler,
+    register_plugin,
+    register_section,
+)
+
+__all__ = [
+    "ConfigManager",
+    "Env",
+    "Environment",
+    "EnvironmentConfig",
+    "config",
+    "get_handlers",
+    "get_sections",
+    "register_handler",
+    "register_plugin",
+    "register_section",
+]
+
+
+def _create_manager() -> ConfigManager:
+    return ConfigManager()
+
+
+_base_manager = _create_manager()
+# TODO(FXC-3827): Drop LegacyConfigWrapper once legacy accessors are removed in Tidy3D 2.12.
+_config_wrapper = LegacyConfigWrapper(_base_manager)
+config = _config_wrapper
+
+# TODO(FXC-3827): Remove legacy Env exports after deprecation window (planned 2.12).
+Environment = LegacyEnvironment
+EnvironmentConfig = LegacyEnvironmentConfig
+Env: LegacyEnvironment | None = None
+
+
+def initialize_env() -> None:
+    """Initialize legacy Env after sections register."""
+
+    global Env
+    if Env is None:
+        Env = LegacyEnvironment(_base_manager)
+
+
+def reload_config(*, profile: str | None = None) -> LegacyConfigWrapper:
+    """Recreate the global configuration manager (primarily for tests)."""
+
+    global _base_manager, Env
+    if _base_manager is not None:
+        try:
+            _base_manager.apply_web_env({})
+        except AttributeError:
+            pass
+    _base_manager = ConfigManager(profile=profile)
+    _config_wrapper.reset_manager(_base_manager)
+    if Env is None:
+        initialize_env()
+    Env.reset_manager(_base_manager)
+    return _config_wrapper
+
+
+def get_manager() -> ConfigManager:
+    """Return the underlying configuration manager instance."""
+
+    return _base_manager
+
+
+def __getattr__(name: str) -> Any:
+    if name == "Env":
+        initialize_env()
+        return Env
+    return getattr(config, name)