Skip to content

feat: add IN [...] coord filter to sum_over#69

Merged
jc-macdonald merged 1 commit intomainfrom
feature/sum-over-subset
Apr 23, 2026
Merged

feat: add IN [...] coord filter to sum_over#69
jc-macdonald merged 1 commit intomainfrom
feature/sum-over-subset

Conversation

@jc-macdonald
Copy link
Copy Markdown
Collaborator

@jc-macdonald jc-macdonald commented Apr 23, 2026

This pull request adds support for filtering the coordinates included in sum_over expressions using an optional IN [...] clause. This allows users to specify a subset of axis coordinates to sum over, rather than always summing over all coordinates. The implementation includes robust error checking for invalid, empty, unknown, or duplicate coordinates in the filter. Comprehensive tests have been added to ensure correct behavior and error handling.

Enhancements to sum_over syntax:

  • Updated the _SUM_OVER_RE regular expression in src/op_system/specs.py to support an optional IN [...] filter for specifying a subset of coordinates to sum over.
  • Added the _apply_coord_filter helper function to validate and apply the coordinate filter, raising errors for empty, unknown, or duplicate coordinates.
  • Modified the _expand_sum_over logic to use the filtered coordinate list when expanding sums, ensuring only the specified coordinates are included.

Testing and validation:

  • Added multiple tests in tests/op_system/test_op_system_specs.py to verify correct expansion and error handling for the new sum_over IN [...] syntax, including cases for valid filters, empty filters, unknown coordinates, and duplicates.
  • Added an end-to-end test in tests/op_system/test_op_system_compile.py to ensure that the filtered sum compiles and evaluates correctly.

Copy link
Copy Markdown
Member

@pearsonca pearsonca left a comment

Choose a reason for hiding this comment

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

Minor notes for future work

Comment thread src/op_system/specs.py
)
_SUM_OVER_RE = re.compile(
r"sum_over\(\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*([A-Za-z_][A-Za-z0-9_]*)\s*,\s*(.*?)\)",
r"sum_over\(\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*([A-Za-z_][A-Za-z0-9_]*)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note for future work: desire to avoid needing regular expressions. I think the ideal here is figuring out how to rely on the equation / symbol parsing libs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what's error like for, say, a missing close ]? we should figure out (perhaps not in this PR) how to handle that sort of problem gracefully and with useful feedback.

@jc-macdonald jc-macdonald merged commit d087af9 into main Apr 23, 2026
5 checks passed
@jc-macdonald jc-macdonald deleted the feature/sum-over-subset branch April 23, 2026 16:12
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