Skip to content

Conversation

@marcorudolphflex
Copy link
Contributor

@marcorudolphflex marcorudolphflex commented Nov 5, 2025

Currently, we only support .drc files for the DRC Runner in the DRC Config in the klayout plugin. This PR simply allows also “lydrc” suffixes in the validator.

Greptile Overview

Updated On: 2025-11-05 11:18:46 UTC

Greptile Summary

Extended KLayout DRC plugin to support .lydrc file format in addition to .drc files. .lydrc files are XML-wrapped DRC runsets that KLayout can process.

Key Changes

  • Added SUPPORTED_DRC_SUFFIXES constant containing both .drc and .lydrc suffixes
  • Updated _validate_drc_runset_filetype validator to accept both file types
  • Improved validation error message to list all supported suffixes
  • The existing regex-based format validation correctly handles both plain .drc and XML-wrapped .lydrc content

Test Coverage

  • Added wrap_drc_to_lydrc helper method to generate XML-wrapped content
  • Parametrized existing tests with drc_file_suffix to test both .drc and .lydrc formats
  • All validation, GDS processing, and Tidy3D object tests now cover both formats

Issues Found

  • Validator docstring still mentions only .drc format (needs update)
  • Missing CHANGELOG entry for this user-facing feature addition

Confidence Score: 4/5

  • Safe to merge with minor documentation updates needed
  • The implementation is simple and correct - adding .lydrc to the supported suffixes set and updating the validator. The regex patterns work correctly for both formats since they search for the DRC commands in the file content regardless of XML wrapping. Comprehensive test coverage added. Score is 4 (not 5) due to outdated docstring and missing CHANGELOG entry per repository guidelines.
  • tidy3d/plugins/klayout/drc/drc.py needs docstring update on line 59, and CHANGELOG.md needs an entry

Important Files Changed

File Analysis

Filename Score Overview
tidy3d/plugins/klayout/drc/drc.py 4/5 Added .lydrc support to DRC validator by updating constant and validation logic; docstring needs update
tests/test_plugins/klayout/drc/test_drc.py 5/5 Added comprehensive test coverage for .lydrc format with XML wrapper helper and parametrized tests

Sequence Diagram

sequenceDiagram
    participant User
    participant DRCRunner
    participant DRCConfig
    participant Validator
    participant KLayout

    User->>DRCRunner: run(source, drc_runset)
    DRCRunner->>DRCConfig: create config
    DRCConfig->>Validator: _validate_drc_runset_filetype(path)
    
    alt file suffix is .drc or .lydrc
        Validator-->>DRCConfig: valid
    else file suffix is other
        Validator-->>DRCConfig: ValidationError
    end
    
    DRCConfig->>Validator: _validate_drc_runset_format(path)
    Validator->>Validator: read file content
    
    alt content matches patterns
        Validator-->>DRCConfig: valid
    else content invalid
        Validator-->>DRCConfig: ValidationError
    end
    
    DRCConfig-->>DRCRunner: validated config
    DRCRunner->>KLayout: execute DRC with runset
    KLayout-->>DRCRunner: results
    DRCRunner-->>User: DRCResults
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.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@marcorudolphflex marcorudolphflex force-pushed the FXC-3982-support-lydrc-in-klayout-plugin branch from 517dffe to 9beae5e Compare November 5, 2025 11:20
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Diff Coverage

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

  • tidy3d/plugins/klayout/drc/drc.py (100%)

Summary

  • Total: 3 lines
  • Missing: 0 lines
  • Coverage: 100%

@marcorudolphflex marcorudolphflex force-pushed the FXC-3982-support-lydrc-in-klayout-plugin branch from 9beae5e to 3924d49 Compare November 5, 2025 11:41
Copy link
Contributor

@bzhangflex bzhangflex left a comment

Choose a reason for hiding this comment

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

LGTM

@marcorudolphflex marcorudolphflex added this pull request to the merge queue Nov 5, 2025
Merged via the queue into develop with commit 8dec0cb Nov 5, 2025
37 checks passed
@marcorudolphflex marcorudolphflex deleted the FXC-3982-support-lydrc-in-klayout-plugin branch November 5, 2025 15:35
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