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-8212: Add support for reading metadata, length and schema of a catalog #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup. This is certainly better than it was. I think it would be even better to break out most/all of this (plus some of the code above) into a separate method, because init is an overly-complex god function right now.
@@ -207,6 +209,31 @@ def testSubMap(self): | |||
'ccd = 13\nheight = 400\nimageOrigin = "PARENT"\n' | |||
'llcX = 200\nllcY = 100\nwidth = 300\n') | |||
|
|||
def testCatalogExtras(self): | |||
butler = dafPersist.ButlerFactory(mapper=self.mapper).create() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need to do this via a ButlerFactor? Can't you just create a butler directly?
del subId['bbox'] | ||
return mapping.lookup(format, subId) | ||
setattr(self, "query_" + datasetType + "_sub", querySubClosure) | ||
def setMethods(suffix, mapImpl=None, bypassImpl=None, queryImpl=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this whole subPolicy block that you've cleanedup/added (basically most of the contents of this inner for loop) should be pulled into a separate _addPolicies
method. __init__
is really long as it stands, and this could help break it up (I didn't even see that this was all in a double-for loop!).
c83eaee
to
f963d06
Compare
Extra dataset features (e.g., "raw_sub", "raw_md", "raw_filename") are are created from the original mapping (e.g., "raw") by defining a bunch of extra methods. This patch replaces the sea of if not hasattr(self, someName): setattr(self, someName, someImpl) calls with a common function to set the implementations for each extra dataset feature. This clarifies the code and makes it easier to add more extra features. Moved the setting of dataset mappings into its own method, as it's specialised and too much to have in __init__.
Added "foo_md", "foo_len" and "foo_schema" if "foo" is a catalog. Note that the "foo_schema" commonly has a different meaning because it's usually defined explicitly if the "foo" is an important pipeline output.
f963d06
to
d33c0b1
Compare
That's cleaner, thanks. I think it would be great if the guts of the inner |
No description provided.