feat(tidy3d): FXC-4258-automate-adding-k-layout-to-the-path-for-this-python-session

[marcorudolphflex]

̶B̶e̶f̶o̶r̶e̶ ̶̶c̶h̶e̶c̶k̶_̶i̶n̶s̶t̶a̶l̶l̶a̶t̶i̶o̶n̶̶ ̶i̶n̶ ̶̶p̶l̶u̶g̶i̶n̶s̶.̶k̶l̶a̶y̶o̶u̶t̶.̶u̶t̶i̶l̶̶,̶ ̶s̶y̶s̶t̶e̶m̶-̶s̶p̶e̶c̶i̶f̶i̶c̶ ̶p̶a̶t̶h̶ ̶a̶t̶ ̶t̶y̶p̶i̶c̶a̶l̶ ̶l̶o̶c̶a̶t̶i̶o̶n̶s̶ ̶i̶s̶ ̶a̶d̶d̶e̶d̶ ̶t̶o̶ ̶P̶A̶T̶H̶ ̶(̶i̶f̶ ̶f̶o̶u̶n̶d̶)̶.̶
̶A̶l̶s̶o̶ ̶e̶n̶s̶u̶r̶e̶d̶ ̶t̶h̶a̶t̶ ̶t̶h̶e̶ ̶a̶c̶t̶u̶a̶l̶ ̶f̶o̶u̶n̶d̶ ̶c̶o̶m̶m̶a̶n̶d̶/̶p̶a̶t̶h̶ ̶f̶r̶o̶m̶ ̶̶w̶h̶i̶c̶h̶̶ ̶i̶s̶ ̶u̶s̶e̶d̶.̶
̶C̶o̶u̶l̶d̶ ̶n̶o̶t̶ ̶t̶e̶s̶t̶ ̶t̶h̶a̶t̶ ̶o̶n̶ ̶W̶i̶n̶d̶o̶w̶s̶,̶ ̶c̶a̶n̶ ̶a̶n̶y̶b̶o̶d̶y̶?̶
̶F̶i̶x̶e̶d̶ ̶t̶h̶e̶ ̶g̶r̶e̶p̶t̶i̶l̶e̶-̶m̶e̶n̶t̶i̶o̶n̶e̶d̶ ̶f̶l̶a̶w̶s̶ ̶o̶f̶ ̶t̶h̶e̶ ̶t̶e̶s̶t̶s̶.̶
̶
̶̶̶̶ ̶f̶r̶o̶m̶ ̶t̶i̶d̶y̶3̶d̶.̶p̶l̶u̶g̶i̶n̶s̶.̶k̶l̶a̶y̶o̶u̶t̶.̶u̶t̶i̶l̶ ̶i̶m̶p̶o̶r̶t̶ ̶c̶h̶e̶c̶k̶_̶i̶n̶s̶t̶a̶l̶l̶a̶t̶i̶o̶n̶ ̶i̶m̶p̶o̶r̶t̶ ̶o̶s̶ ̶o̶s̶.̶e̶n̶v̶i̶r̶o̶n̶[̶"̶P̶A̶T̶H̶"̶]̶ ̶=̶ ̶"̶"̶ ̶c̶h̶e̶c̶k̶_̶i̶n̶s̶t̶a̶l̶l̶a̶t̶i̶o̶n̶(̶r̶a̶i̶s̶e̶_̶e̶r̶r̶o̶r̶=̶T̶r̶u̶e̶)̶ ̶̶̶̶
̶T̶h̶i̶s̶ ̶s̶h̶o̶u̶l̶d̶ ̶p̶a̶s̶s̶ ̶i̶f̶ ̶k̶l̶a̶y̶o̶u̶t̶ ̶i̶n̶s̶t̶a̶l̶l̶e̶d̶ ̶a̶n̶d̶ ̶s̶h̶o̶u̶l̶d̶ ̶n̶o̶t̶ ̶p̶a̶s̶s̶ ̶i̶f̶ ̶y̶o̶u̶ ̶r̶e̶m̶o̶v̶e̶ ̶t̶h̶e̶ ̶̶_̶e̶n̶s̶u̶r̶e̶_̶c̶o̶m̶m̶o̶n̶_̶i̶n̶s̶t̶a̶l̶l̶_̶l̶o̶c̶a̶t̶i̶o̶n̶s̶_̶o̶n̶_̶p̶a̶t̶h̶(̶)̶̶ ̶i̶n̶ ̶̶c̶h̶e̶c̶k̶_̶i̶n̶s̶t̶a̶l̶l̶a̶t̶i̶o̶n̶̶.̶
We decided to just run the found executable instead of the unnecessarily PATH manipulation.

Fixed 2 more tiny bugs: Convert config.drc_runset safely to str when executing cmd. Also, take the absolute resultsfile path as klayout will interpret it relatively to its own location.

Greptile Overview

Greptile Summary

Automatically adds KLayout installation directories to PATH when using the klayout plugin, improving user experience by removing the manual PATH configuration requirement.

Major Changes:

• Added platform-specific detection for common KLayout installation paths (macOS app bundles, Windows Program Files, Linux standard locations)
• Modified check_installation() to automatically prepend discovered KLayout directories to PATH before checking
• Refactored to use the resolved klayout executable path instead of hardcoded "klayout" command
• Moved and enhanced tests for klayout utility functions to dedicated test file

Issues Found:

• Test test_check_klayout_not_installed has a critical bug where it doesn't monkeypatch _common_install_locations, which will cause the test to fail if KLayout is installed at a common location on the test machine
• Test test_check_klayout_installed should also monkeypatch _common_install_locations for consistency and robustness

Confidence Score: 3/5

• This PR has a critical test bug that will cause failures in certain CI environments, but the core functionality is solid
• The implementation of automatic PATH detection is well-designed and handles multiple platforms correctly. However, the test for the negative case (klayout not installed) has a logic bug that will cause it to fail if KLayout is actually installed on the test machine at common locations. This is a critical issue for CI reliability.
• Pay close attention to tests/test_plugins/klayout/test_util.py - the test monkeypatching is incomplete

Important Files Changed

File Analysis

Filename	Score	Overview
tidy3d/plugins/klayout/util.py	4/5	Added automatic PATH detection for KLayout installations across platforms (macOS, Windows, Linux); implementation looks solid but has minor style considerations
tests/test_plugins/klayout/test_util.py	3/5	New test file with moved and new tests; tests have a fragility issue where they don't monkeypatch all dependencies

Sequence Diagram

sequenceDiagram
    participant User
    participant check_installation
    participant _ensure_common_install_locations_on_path
    participant _resolve_klayout_executable
    participant _common_install_locations
    participant _prepend_to_path
    participant which

    User->>check_installation: call with raise_error=True
    check_installation->>_ensure_common_install_locations_on_path: ensure PATH is set up
    _ensure_common_install_locations_on_path->>_resolve_klayout_executable: check if already on PATH
    _resolve_klayout_executable->>which: check "klayout" (or "klayout_app.exe" on Windows)
    which-->>_resolve_klayout_executable: None (not found)
    _resolve_klayout_executable-->>_ensure_common_install_locations_on_path: None
    _ensure_common_install_locations_on_path->>_common_install_locations: get platform-specific paths
    _common_install_locations-->>_ensure_common_install_locations_on_path: list of Path objects
    loop for each binary path
        _ensure_common_install_locations_on_path->>_ensure_common_install_locations_on_path: check if binary.exists()
        alt binary exists
            _ensure_common_install_locations_on_path->>_prepend_to_path: add binary.parent to PATH
            _prepend_to_path->>_prepend_to_path: update os.environ["PATH"]
            _prepend_to_path-->>_ensure_common_install_locations_on_path: done
        end
    end
    _ensure_common_install_locations_on_path-->>check_installation: PATH updated
    check_installation->>_resolve_klayout_executable: resolve executable
    _resolve_klayout_executable->>which: check again
    which-->>_resolve_klayout_executable: /Applications/KLayout.app/Contents/MacOS/klayout
    _resolve_klayout_executable-->>check_installation: path to klayout
    check_installation-->>User: return klayout path

Loading

[greptile-apps]

_{5 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}