Skip to content

Conversation

dmarek-flex
Copy link
Contributor

Strange validator errors reported here. So we need better validatation for the CustomGridBoundaries

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.

PR Summary

Enhanced validation for CustomGridBoundaries to prevent invalid grid specifications and improve error messaging.

  • Added coordinate array validation in tidy3d/components/grid/grid_spec.py requiring minimum 2 entries and ascending order
  • Implemented new test cases in tests/test_components/test_grid_spec.py to verify sorting and length requirements
  • Fixed validator errors reported in SCRF-750 by adding explicit boundary checks for custom grid coordinates

3 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

@dmarek-flex dmarek-flex requested a review from tylerflex June 11, 2025 14:05
Copy link
Collaborator

@tylerflex tylerflex left a comment

Choose a reason for hiding this comment

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

looks good, thanks @dmarek-flex. I'd say that adding the indices like the bot says is probably slightly better and might help in the long run. once that's in feel free to merge.

Copy link
Contributor

github-actions bot commented Jun 11, 2025

Diff Coverage

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

  • tidy3d/components/grid/grid_spec.py (100%)

Summary

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

@dmarek-flex dmarek-flex self-assigned this Jun 11, 2025
@dmarek-flex dmarek-flex force-pushed the dmarek/fix-custom-grid-validation branch from 1092ab1 to a752f06 Compare June 11, 2025 19:22
@dmarek-flex dmarek-flex merged commit 0157ce9 into develop Jun 11, 2025
23 checks passed
@dmarek-flex dmarek-flex deleted the dmarek/fix-custom-grid-validation branch June 11, 2025 19:56
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.

2 participants