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-7976 fix reading old coadds (CoaddInputs/ExposureTable) #104

Merged
merged 3 commits into from Oct 15, 2016

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented Oct 14, 2016

No description provided.

daf::base::PropertySet md;
auto nullVisitInfoPtr = std::make_shared<image::VisitInfo>(md);
output.setVisitInfo(nullVisitInfoPtr);
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little surprised you consider it better to default-construct a VisitInfo here rather than leave it an empty pointer. Is having a non-null VisitInfo required for all Exposure objects (even coadd?!) and hence something you want to enforce for ExposureRecords as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to leave it an empty pointer. How do I do that?

Copy link
Member

Choose a reason for hiding this comment

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

If you simply do nothing, it should already be set to an empty pointer. If you want to make it explicit (for readers of the code), you could do:

output.setVisitInfo(nullptr);

I'm ambivalent about which is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I'll make it explicit. A big improvement in any case.

@r-owen r-owen changed the title Tickets/dm 7976 DM-7976 fix reading old CoaddInputs data Oct 14, 2016
@r-owen r-owen changed the title DM-7976 fix reading old CoaddInputs data DM-7976 fix reading old coadds (CoaddInputs/ExposureTable) Oct 14, 2016
@r-owen r-owen merged commit 9e5fb29 into master Oct 15, 2016
@ktlim ktlim deleted the tickets/DM-7976 branch August 25, 2018 06:44
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

2 participants