feat(parsers): Add __eq__ and __hash__ to Param#80
Merged
ericchansen merged 2 commits intomasterfrom Mar 20, 2026
Merged
Conversation
Parameter identity is defined by (ptype, ff_row, ff_col). This enables set operations, deduplication, and cleaner test assertions. Includes comprehensive test suite for equality, construction, angle normalization, and range validation (19 new tests). Closes #78 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds value-based identity semantics to q2mm.parsers.param.Param so parameters can be compared/deduplicated and used as set/dict keys, with a new pytest suite covering equality/hashing and existing value normalization/range behavior.
Changes:
- Implement
Param.__eq__andParam.__hash__using(ptype, ff_row, ff_col)as the identity tuple. - Add
test/test_param.pywith tests for equality/hashing, set behavior, construction, angle normalization, and range validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| q2mm/parsers/param.py | Adds __eq__/__hash__ to make Param comparable and hashable by FF position. |
| test/test_param.py | Introduces a pytest suite validating the new equality/hashing behavior and related Param behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Params with None in (ptype, ff_row, ff_col) now: - Return NotImplemented from __eq__ (falls back to identity) - Raise TypeError from __hash__ (prevents accidental set collisions) - Add self-identity fast path (self is other) - Document mutability constraint on identity fields in __hash__ docstring Adds 3 new tests for edge cases (22 total in test_param.py). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
__eq__and__hash__toParamso parameters can be compared, deduplicated, and used in sets.Changes
q2mm/parsers/param.py-- Add__eq__and__hash__based on(ptype, ff_row, ff_col)identity tupletest/test_param.py-- New test suite (19 tests) covering equality, hashing, set operations, construction, angle normalization, and range validationWhy
The original authors noted this was needed ("Need a general index scheme to compare the equalness of two parameters"). Without
__eq__, comparing params required manualff_row/ff_colchecks, and params couldn't be used in sets or as dict keys.Testing
Closes #78