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-14359: Fix data ID handling in ap_* #93

Merged
merged 1 commit into from May 14, 2018
Merged

Conversation

kfindeisen
Copy link
Member

This PR factors some of the dataId handling logic out of pipe_base so that it can be used by code other than ArgumentParser. I've created a new module, butlerHelpers.py, for similar application logic that may be factored out in the future (in the unlikely event that it's needed before we rewrite everything for Butler 3).

Copy link
Contributor

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

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

Thanks for adding these here. I'll figure out how to use them in jointcal's plotting code.

Please clarify why dataExists needs to be recursive.

subDRList = dataRef.subItems()
if subDRList:
for subDR in subDRList:
if dataExists(subDR):
Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious why this needs to be recursive. Wouldn't just datasetExists handlle all such situations? When does it not?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea. This is just copied from pipe_base.

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like @r-owen wrote this function originally.

The type of data references to return.
level : `str`
The level of data ID at which to search. If the empty string, the
default level for ``datasetType`` shall be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please reference wherever it is in the butler docs that level is better defined. If anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not aware of any definition of level in the docs.

Copy link
Contributor

@r-owen r-owen May 11, 2018

Choose a reason for hiding this comment

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

I would be surprised if you could find appropriate docs. I suggest we live with it. It was necessary in the butler when I wrote it, because of dataset types such as LSST raw images, which were per amplifier. I strongly suspect it's still necessary (though you were welcome to try removing it and see if we can still process LSST phosim data). It will go away with gen3 butler, thank heavens.

@kfindeisen kfindeisen merged commit 9c54c7d into master May 14, 2018
@ktlim ktlim deleted the tickets/DM-14359 branch August 25, 2018 06:14
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