DM-25000: Make registry query methods work with composition again. #289
Conversation
This file contains 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
We stopped using these code paths outside of tests on DM-24288, and removing them is a huge simplification for Registry. In detail: - DatasetRef.components has been removed, along with the flatten and allRefs methods. This means DatasetRefs now only have two states (resolved and unresolved), instead of it being unclear whether an empty dict means there are no components or that we don't know about the components. DatasetRef.makeComponentRef has been added to make it easy to make a ref for a component dataset from the parent ref. - All "recursive" kwargs have been removed, along with some occasionally-complex logic needed to implement them. - The dataset_composition table has been removed. It is worth noting that I found a couple of spots where we were still calling insertDatasets with recursive=True (ingest and import from yaml). So I think we were still generating unnecessary database rows sometimes, including in contexts where a lot of datasets were involved. I don't think those rows were ever used after insertion, which is why this didn't break anything.
We now just implement __repr__ and let __str__ delegate to that, and aim to make it informative and non-confusing because we've never been able to make the string eval'able as in the ideal case. Note that the biggest problem with the old repr (which is what is used by __str__ in built-in containers of data IDs, regardless of what we'd prefer) is that it printed the full DimensionGraph, which often has implied dimensions and hence more "keys" than the mapping interface actually provided access to. That it impossible to infer what the dict-like form actually was, and it's the dict-like form that people actually want to know.
We now have an option to control whether components are included in the results. We expect users to consider them noise if the parent is also included in the results, so the default is now to not include them unless the parent is not in the results.
TallJimbo
force-pushed
the
tickets/DM-25000
branch
from
cee2c98
to
f7999e9
Compare
timj
approved these changes
May 21, 2020
| childType = registry.getDatasetType("permabias.wcs") | ||
| parentRefResolved = registry.findDataset(parentType, collections=collection, | ||
| instrument="Cam1", detector=1) | ||
| self.assertIsNotNone(parentRefResolved) |
There was a problem hiding this comment.
Maybe we could be more positive in this test and use:
self.assertIsInstance(parentRefResolved, DatasetRef)
?
| parentRefUnresolved = parentRefResolved.unresolved() | ||
| # Search for a single dataset with findDataset. | ||
| childRef1 = registry.findDataset("permabias.wcs", collections=collection, | ||
| dataId=parentRefUnresolved.dataId) |
There was a problem hiding this comment.
Isn't the dataId of parentRefUnresolved the same as the dataId for parentRefResolved?
There was a problem hiding this comment.
Good catch; the unresolved variable's existence is a relic of a previous incarnation of the test, and I've removed it.
TallJimbo
force-pushed
the
tickets/DM-25000
branch
from
97367b7
to
11c8da9
Compare
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.
No description provided.