Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-26371: Make Quantum immutable #382

Merged
merged 1 commit into from Sep 28, 2020
Merged

DM-26371: Make Quantum immutable #382

merged 1 commit into from Sep 28, 2020

Conversation

natelust
Copy link
Contributor

Remove all the properties of Quantum that were expected to be
mutable. Make sure object was then immutable. Add a hash and
eq function based on immutable instance attributes.

@timj timj changed the title tickets/DM-26371 Make Quantum immutable DM-26371: Make Quantum immutable Sep 24, 2020
@@ -165,7 +131,7 @@ def initInputs(self) -> NamedKeyDict[DatasetType, DatasetRef]:
with `DatasetType` instances as keys (names can also be used for
lookups) and `DatasetRef` instances as values.
"""
return self._initInputs
return copy.copy(self._initInputs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out NamedKeyDict.freeze; I think it's exactly what you want here and in similar accessors (or maybe the constructor). That'd come with changing the return type NamedKeyMapping[DatasetType, DatasetRef].

Copy link
Contributor

@andy-slac andy-slac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an improvement, but I'm concerned again with the hash function, couple of questions related to that.

python/lsst/daf/butler/core/quantum.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/quantum.py Outdated Show resolved Hide resolved
@@ -99,39 +84,26 @@ class Quantum:
def __init__(self, *, taskName: Optional[str] = None,
taskClass: Optional[Type] = None,
dataId: Optional[DataCoordinate] = None,
run: Optional[str] = None,
initInputs: Optional[Union[Mapping[DatasetType, DatasetRef], Iterable[DatasetRef]]] = None,
predictedInputs: Optional[Mapping[DatasetType, List[DatasetRef]]] = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With dropping actualInputs would it make sense to call it inputs instead of predictedInputs?

Remove all the properties of Quantum that were expected to be
mutable. Make sure object was then immutable. Add a hash and
eq function based on immutable instance attributes.
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.

None yet

3 participants