Skip to content

Fix ArrayValue.key to include hash of full array - #51

Open
xovishnukosuri wants to merge 1 commit into
INCF:masterfrom
xovishnukosuri:fix/array-value-key-hash
Open

xovishnukosuri wants to merge 1 commit into
INCF:masterfrom
xovishnukosuri:fix/array-value-key-hash

Conversation

@xovishnukosuri

Copy link
Copy Markdown

Problem

ArrayValue.key only 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. Since AnnotatedNineMLObject._add_member uses element.key as a dict key, this causes silent collisions where distinct ArrayValue objects 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.

@property
def key(self):
    prefix = '_'.join(str(v) for v in self._values[:10])
    values_str = str(list(self._values))
    return '{}_{}'.format(prefix, hash(values_str))

The prefix is kept so keys remain human-readable when debugging.

Tests

Three new tests in test/unittests/exception_test/test_values.py:

  • arrays sharing the first 10 values but differing after position 10 get distinct keys
  • two ArrayValue objects built from identical data get the same key
  • a short array does not collide with a longer array that starts with the same values

All 11 tests in the file pass locally on Python 3.13.

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>
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.

Add hash to ArrayValue key

1 participant