Fix ArrayValue.key to include hash of full array - #51
Open
xovishnukosuri wants to merge 1 commit into
Open
xovishnukosuri wants to merge 1 commit into
xovishnukosuri wants to merge 1 commit into
Conversation
The key property only used the first 10 values joined by underscores, so two arrays sharing the same first 10 values but differing elsewhere produced the same key. This caused incorrect dict collisions in AnnotatedNineMLObject._add_member. The fix appends a hash of the complete value list to make the key unique. Closes INCF#36 Co-Authored-By: Claude Sonnet 4.6 <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
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.
Problem
ArrayValue.keyonly joined the first 10 values with underscores. Two arrays that share the same first 10 elements but differ after that position (or in length) produce an identical key. SinceAnnotatedNineMLObject._add_memberuseselement.keyas a dict key, this causes silent collisions where distinctArrayValueobjects overwrite each other in containers.The TODO comment in the code acknowledged this:
# TODO: Should put a hash on the end of this to make it unique.Closes #36.
Fix
Append
hash(str(list(self._values)))to the prefix so the key covers the entire array.The prefix is kept so keys remain human-readable when debugging.
Tests
Three new tests in
test/unittests/exception_test/test_values.py:ArrayValueobjects built from identical data get the same keyAll 11 tests in the file pass locally on Python 3.13.