Skip to content

Commit

Permalink
Merge pull request #1044 from lsst/tickets/DM-45517
Browse files Browse the repository at this point in the history
DM-45517: No longer check parent when checking compatibility
  • Loading branch information
parejkoj committed Aug 1, 2024
2 parents 28efc2a + 7570ecb commit 106dc9f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ repos:
name: isort (python)
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.5.4
rev: v0.5.5
hooks:
- id: ruff
- repo: https://github.com/numpy/numpydoc
rev: "v1.7.0"
rev: "v1.8.0rc2"
hooks:
- id: numpydoc-validation
15 changes: 10 additions & 5 deletions python/lsst/daf/butler/_dataset_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,7 @@ def _equal_ignoring_storage_class(self, other: Any) -> bool:
return False
if self._isCalibration != other._isCalibration:
return False
if self._parentStorageClass is not None and other._parentStorageClass is not None:
return self._parentStorageClass == other._parentStorageClass
else:
return self._parentStorageClassName == other._parentStorageClassName
return True

def __eq__(self, other: Any) -> bool:
mostly_equal = self._equal_ignoring_storage_class(other)
Expand All @@ -296,6 +293,13 @@ def __eq__(self, other: Any) -> bool:

# Be careful not to force a storage class to import the corresponding
# python code.
if self._parentStorageClass is not None and other._parentStorageClass is not None:
if self._parentStorageClass != other._parentStorageClass:
return False
else:
if self._parentStorageClassName != other._parentStorageClassName:
return False

if self._storageClass is not None and other._storageClass is not None:
if self._storageClass != other._storageClass:
return False
Expand All @@ -309,7 +313,8 @@ def is_compatible_with(self, other: DatasetType) -> bool:
Compatibility requires a matching name and dimensions and a storage
class for this dataset type that can convert the python type associated
with the other storage class to this python type.
with the other storage class to this python type. Parent storage class
compatibility is not checked at all for components.
Parameters
----------
Expand Down
20 changes: 16 additions & 4 deletions tests/test_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def testCompatibility(self) -> None:

def testOverrideStorageClass(self) -> None:
storageA = StorageClass("test_a", pytype=list, converters={"dict": "builtins.list"})
storageB = StorageClass("test_b", pytype=dict)
storageB = StorageClass("test_b", pytype=dict, converters={"list": "dict"})
dimensions = self.universe.conform(["instrument"])

dA = DatasetType("a", dimensions, storageA)
Expand All @@ -291,15 +291,27 @@ def testOverrideStorageClass(self) -> None:
round_trip = dB.overrideStorageClass(storageA)
self.assertEqual(round_trip, dA)

# Check that parents move over.
parent = StorageClass("composite", components={"a": storageA, "c": storageA})
# Check that parents move over. Assign a pytype to avoid using
# object in later tests.
parent = StorageClass("composite", pytype=tuple, components={"a": storageA, "c": storageA})
dP = DatasetType("comp", dimensions, parent)
dP_A = dP.makeComponentDatasetType("a")
print(dP_A)
dp_B = dP_A.overrideStorageClass(storageB)
self.assertEqual(dp_B.storageClass, storageB)
self.assertEqual(dp_B.parentStorageClass, parent)

# Check that components are checked for compatibility but parents
# can be different.
parent2 = StorageClass(
"composite2",
pytype=frozenset,
components={"a": storageB, "c": storageB},
)
dP2 = DatasetType("comp", dimensions, parent2)
# Components are compatible even though parents aren't.
self.assertFalse(dP.is_compatible_with(dP2))
self.assertTrue(dP2.makeComponentDatasetType("a").is_compatible_with(dP_A))

def testJson(self) -> None:
storageA = StorageClass("test_a")
dimensionsA = self.universe.conform(["instrument"])
Expand Down
14 changes: 14 additions & 0 deletions tests/test_parquet.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,20 @@ def testWriteSingleIndexDataFrameReadAsArrowTable(self):
# We check the set because pandas reorders the columns.
self.assertEqual(set(columns), set(columns2))

# Override the component using a dataset type.
columnsType = self.datasetType.makeComponentDatasetType("columns").overrideStorageClass(
"ArrowColumnList"
)
self.assertEqual(columns2, self.butler.get(columnsType))

# Check getting a component while overriding the storage class via
# the dataset type. This overrides the parent storage class and then
# selects the component.
columnsType = self.datasetType.overrideStorageClass("ArrowAstropy").makeComponentDatasetType(
"columns"
)
self.assertEqual(columns2, self.butler.get(columnsType))

# Check reading the schema.
schema = tab2.schema
schema2 = self.butler.get(
Expand Down

0 comments on commit 106dc9f

Please sign in to comment.