Tickets/dm 8213 #120
Tickets/dm 8213 #120
Conversation
| /// | ||
| /// Includes support for the INHERIT convention: if 'INHERIT = T' is in the header, the | ||
| /// PHU will be read as well, and nominated HDU will override any duplicated values. | ||
| PTR(daf::base::PropertyList) readMetadata(std::string const & fileName, int hdu=0, bool strip=false); |
There was a problem hiding this comment.
This seems like a very logical set of new functions.
I'm a bit disappointed that we can't just return a PropertyList instead of a pointer to a property list. I assume the pointer is used to avoid copying the data (though I wonder if C++ really would copy in this situation, or if it could swap).
Please document all three functions independently, so they all show up as documented in the Doxygen. If you want to void repetition then I suppose you could fully document one version and have a one-liner for the others that points to the full docs of the one, but personally I'd just add the repetition.
There was a problem hiding this comment.
As I understand it, the //@{ at the top of the doc and the //@} below the last declaration of readMetadata group the declarations together, so they all share the same doc.
The pointer is used because that's what we had before and I didn't want to change it without thinking more carefully.
| } | ||
|
|
||
| PTR(daf::base::PropertyList) readMetadata(fits::Fits & fitsfile, bool strip) { | ||
| PTR(daf::base::PropertyList) metadata(new lsst::daf::base::PropertyList); |
There was a problem hiding this comment.
Is there's less repetitive way way to do this (and similar code below) with auto? Would this work?:
auto readMetadata = std::make_shared<daf::base::PropertyList>();
This version returns a PropertyList instead of a PropertySet. Deprecated afw::image::readMetadata (it's in the wrong namespace) and converted it to use afw::fits::readMetadata.
PaulPrice
force-pushed
the
tickets/DM-8213
branch
from
06382ec
to
fbb3f97
Compare
No description provided.