daf_persistence: make Butler.datasetExists work with composite datasets #62
Conversation
| if isinstance(location, ButlerComposite): | ||
| for name, componentInfo in location.componentInfo.items(): | ||
| if componentInfo.subset: | ||
| subset = self.subset(datasetType=componentInfo.datasetType, dataId=location.dataId) |
There was a problem hiding this comment.
PEP8: line above is too long - consider moving dataId... to next line.
There was a problem hiding this comment.
we allow 110, this is 104.
| location = None | ||
|
|
||
| return bool(location) | ||
| exists = self.datasetExists(componentInfo.datasetType, location.dataId) |
There was a problem hiding this comment.
The indentation looks odd. I believe that we are sticking to the 110 line limit; how long is this one?
There was a problem hiding this comment.
@RobertLuptonTheGood are you reading the code on the Conversation tab? ...Don't compare the red indentation to the green indentation. If you look at this line under View Changes you'll see the indentation is correct.
@kennywlo the line is 92 columns, < 110.
| if location and location.repository.exists(location): | ||
| break | ||
| exists = None | ||
| if isinstance(location, ButlerComposite): |
There was a problem hiding this comment.
I find ambiguity in the name "ButlerComposite" and its relation to location in the comparison, especially the first part "Butler", in the local context - a comment would help.
|
|
||
| return bool(location) | ||
| exists = self.datasetExists(componentInfo.datasetType, location.dataId) | ||
| if exists is False: |
There was a problem hiding this comment.
I've been shot down for that before; "the double negative is hard to parse"
| location = repoData.repo.map(datasetType, dataId) | ||
| if location and location.repository.exists(location): | ||
| break | ||
| exists = None |
There was a problem hiding this comment.
suggestion: exists = False (given exists being boolean)
|
@kennywlo Please re-review the changes under the latest commit. |
No description provided.