Fix __hash__/__eq__ inconsistency in HConfigChild (#185)#236
Merged
Conversation
__hash__ included new_in_config and order_weight but __eq__ intentionally excluded them, violating the Python invariant that a == b implies hash(a) == hash(b). __eq__ also checked tags but __hash__ did not include them. Align __hash__ to use the same fields as __eq__: text, tags, and children. Add five tests covering each dimension of the inconsistency and its practical impact on set deduplication and dict key lookup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jtdub
force-pushed
the
issue-185-fix-hash-eq-inconsistency-next
branch
from
98b22c1 to
fbcf558
Compare
- test_benchmarks.py: replace append loops with extend (PERF401), add @staticmethod to methods that don't use self (PLR6301), suppress intentional print calls with noqa: T201 - test_child.py: suppress pylint too-many-lines (C0302) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jtdub
added a commit
that referenced
this pull request
Mar 26, 2026
* Fix __hash__/__eq__ inconsistency in HConfigChild (#185) __hash__ included new_in_config and order_weight but __eq__ intentionally excluded them, violating the Python invariant that a == b implies hash(a) == hash(b). __eq__ also checked tags but __hash__ did not include them. Align __hash__ to use the same fields as __eq__: text, tags, and children. Add five tests covering each dimension of the inconsistency and its practical impact on set deduplication and dict key lookup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing lint errors in next branch - test_benchmarks.py: replace append loops with extend (PERF401), add @staticmethod to methods that don't use self (PLR6301), suppress intentional print calls with noqa: T201 - test_child.py: suppress pylint too-many-lines (C0302) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
jtdub
added a commit
that referenced
this pull request
Mar 26, 2026
* Fix __hash__/__eq__ inconsistency in HConfigChild (#185) __hash__ included new_in_config and order_weight but __eq__ intentionally excluded them, violating the Python invariant that a == b implies hash(a) == hash(b). __eq__ also checked tags but __hash__ did not include them. Align __hash__ to use the same fields as __eq__: text, tags, and children. Add five tests covering each dimension of the inconsistency and its practical impact on set deduplication and dict key lookup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing lint errors in next branch - test_benchmarks.py: replace append loops with extend (PERF401), add @staticmethod to methods that don't use self (PLR6301), suppress intentional print calls with noqa: T201 - test_child.py: suppress pylint too-many-lines (C0302) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
jtdub
added a commit
that referenced
this pull request
Mar 26, 2026
* Add CODEOWNERS file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Literal type constraint for cisco_style_text() style parameter (#189) (#240) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Drop v2 migration utilities, rename to load_driver_rules/load_tag_rules, and reorganize tests (#221) Remove v2-to-v3 platform mapping functions and constants. Rename load_hconfig_v2_options to load_driver_rules and load_hconfig_v2_tags to load_tag_rules, preserving dict-based driver extension for Nautobot Golden Config compatibility. Reorganize test suite into unit/, integration/, and benchmarks/ directories mirroring the source code structure. Split the 2079-line test_hier_config.py into focused files by module (test_root.py, test_child.py, test_children.py). Separate driver remediation scenario tests into integration/ and unit tests into unit/platforms/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix load_tag_rules: use _collect_match_rules and correct return type Replace inline match-rule collection with existing _collect_match_rules helper for consistency. Simplify return type from tuple[TagRule] | tuple[TagRule, ...] to tuple[TagRule, ...]. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * update gha to test against the next branch * Fix __hash__/__eq__ inconsistency in HConfigChild (#185) (#236) * Fix __hash__/__eq__ inconsistency in HConfigChild (#185) __hash__ included new_in_config and order_weight but __eq__ intentionally excluded them, violating the Python invariant that a == b implies hash(a) == hash(b). __eq__ also checked tags but __hash__ did not include them. Align __hash__ to use the same fields as __eq__: text, tags, and children. Add five tests covering each dimension of the inconsistency and its practical impact on set deduplication and dict key lookup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing lint errors in next branch - test_benchmarks.py: replace append loops with extend (PERF401), add @staticmethod to methods that don't use self (PLR6301), suppress intentional print calls with noqa: T201 - test_child.py: suppress pylint too-many-lines (C0302) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * Move Huawei VRP tests to integration test directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Rename inconsistent public APIs (#216) - tags_add()/tags_remove() → add_tags()/remove_tags() - cisco_style_text() → indented_text() - dump_simple() → to_lines() - config_to_get_to() → remediation() - depth() method → depth property - Rename private helpers _config_to_get_to/_left/_right accordingly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add custom exception hierarchy (#219) (#239) Add HierConfigError as the base exception with DriverNotFoundError, InvalidConfigError, IncompatibleDriverError, and reparent DuplicateChildError under it. Replace generic ValueError/TypeError raises in constructors and workflows with specific exception types. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add Literal type constraint for indented_text() style parameter (#189) (#241) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.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
1 participant
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
__hash__with__eq__inHConfigChildby removingnew_in_configandorder_weight(intentionally excluded from__eq__) and addingtags(which__eq__checks but__hash__previously omitted)Fixes #185.
Test plan
test_child_hash_eq_consistency_new_in_config— equal children with differingnew_in_confignow have equal hashestest_child_hash_eq_consistency_order_weight— equal children with differingorder_weightnow have equal hashestest_child_hash_eq_consistency_tags— unequal children (differingtags) now have different hashestest_child_set_deduplication_with_new_in_config— equal children collapse to one entry in a settest_child_dict_key_lookup_with_order_weight— equal children resolve to the same dict key🤖 Generated with Claude Code