Apply suggestions from code review

This commit is contained in:
Zac Hatfield-Dodds 2023-10-08 10:00:09 -07:00 committed by GitHub
parent 053745501f
commit 971a037d7f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 5 additions and 2 deletions

View File

@ -1,2 +1,4 @@
In this, we propose a solution that ensures the generation of unique identifiers for ParameterSets in the make_unique_parameterset_ids method in Python.py class. It appends numeric suffixes to identifiers, ensuring uniqueness even when the original identifiers have digit endings.
If the id ends in a digit, make then new id f"{id}_{suffix}". If the new id is already in the set of seen ids, increment the suffix number and try again. This is sure to work, and even be fast in practice, because our maximum suffix is the number of duplicate ids we started with.
Parametrized tests now *really do* ensure that the ids given to each input are unique - for
example, ``a, a, a0`` now results in ``a1, a2, a0`` instead of the previous (buggy) ``a0, a1, a0``.
This necessarily means changing nodeids where these were previously colliding, and for
readability adds an underscore when non-unique ids end in a number.

View File

@ -1011,6 +1011,7 @@ class IdMaker:
new_id = f"{id}{suffix}{id_suffixes[id]}"
resolved_ids[index] = new_id
id_suffixes[id] += 1
assert len(resolved_ids) == len(set(resolved_ids)), f"Internal error: {resolved_ids=}"
return resolved_ids
def _resolve_ids(self) -> Iterable[str]: