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

Tickets/dm 8213 #120

Merged
merged 3 commits into from
Nov 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions include/lsst/afw/fits.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <boost/format.hpp>

#include "lsst/base.h"
#include "lsst/pex/exceptions.h"
#include "lsst/daf/base.h"
#include "ndarray.h"
Expand Down Expand Up @@ -510,6 +511,19 @@ class Fits {

#endif // !SWIG

//@{
/// @brief Read FITS header
///
/// If 'strip' is true, common FITS keys that usually have non-metadata intepretations
/// (e.g. NAXIS, BITPIX) will be ignored.
///
/// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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::MemFileManager & manager, int hdu=0, bool strip=false);
PTR(daf::base::PropertyList) readMetadata(fits::Fits & fitsfile, bool strip=false);
//@}

}}} /// namespace lsst::afw::fits

#endif // !LSST_AFW_fits_h_INCLUDED
1 change: 0 additions & 1 deletion include/lsst/afw/image/Defect.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include <limits>
#include <vector>
#include "lsst/afw/geom.h"
#include "lsst/afw/image/Utils.h"

namespace lsst {
namespace afw {
Expand Down
1 change: 0 additions & 1 deletion include/lsst/afw/image/Image.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
#include "lsst/afw/geom/Box.h"
#include "lsst/afw/geom/Point.h"
#include "lsst/afw/image/lsstGil.h"
#include "lsst/afw/image/Utils.h"
#include "lsst/afw/image/ImageUtils.h"
#include "lsst/afw/math/Function.h"
#include "lsst/daf/base.h"
Expand Down
1 change: 0 additions & 1 deletion include/lsst/afw/image/ImageAlgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include <memory>

#include "lsst/afw/image/lsstGil.h"
#include "lsst/afw/image/Utils.h"
#include "lsst/afw/image/ImageUtils.h"
#include "lsst/daf/base.h"
#include "lsst/daf/base/Citizen.h"
Expand Down
8 changes: 6 additions & 2 deletions include/lsst/afw/image/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,24 @@
#include "lsst/daf/base.h"
#include "lsst/daf/base/Citizen.h"
#include "lsst/pex/exceptions.h"
#include "lsst/afw/formatters/ImageFormatter.h"
#include "lsst/afw/fits.h"

namespace lsst { namespace afw { namespace image {

/**
* @brief Return the metadata (header entries) from a FITS file.
*
* @deprecated Use lsst::afw::fits::readMetadata instead.
*
* @param[in] fileName File to read.
* @param[in] hdu HDU to read, 1-indexed. The special value of 0 will read the
* first non-empty HDU.
* @param[in] strip If true, ignore special header keys usually managed by cfitsio
* (e.g. NAXIS).
*/
PTR(daf::base::PropertySet) readMetadata(std::string const & fileName, int hdu=0, bool strip=false);
inline PTR(daf::base::PropertyList) readMetadata(std::string const & fileName, int hdu=0, bool strip=false) {
return afw::fits::readMetadata(fileName, hdu, strip);
}

/**
* Return a value indicating a bad pixel for the given Image type
Expand Down
2 changes: 2 additions & 0 deletions python/lsst/afw/fits/fitsLib.i
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Python interface to lsst::afw::fits exception classes
#undef SWIG_PYTHON_2_UNICODE
#define SWIG_PYTHON_STRICT_BYTE_CHAR 1
#include "lsst/afw/fits.h"
#include "lsst/daf/base.h"
#include "lsst/pex/exceptions.h"
%}

Expand All @@ -24,6 +25,7 @@ Python interface to lsst::afw::fits exception classes
%naturalvar; // use const reference typemaps

%import "lsst/pex/exceptions/exceptionsLib.i"
%import "lsst/daf/base/baseLib.i"

%include "lsst/afw/fits.h"

Expand Down
41 changes: 41 additions & 0 deletions src/fits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,47 @@ void Fits::closeFile() {
fits_close_file(reinterpret_cast<fitsfile*>(fptr), &status);
}


PTR(daf::base::PropertyList) readMetadata(std::string const & fileName, int hdu, bool strip) {
fits::Fits fp(fileName, "r", fits::Fits::AUTO_CLOSE | fits::Fits::AUTO_CHECK);
fp.setHdu(hdu);
return readMetadata(fp, strip);
}

PTR(daf::base::PropertyList) readMetadata(fits::MemFileManager & manager, int hdu, bool strip) {
fits::Fits fp(manager, "r", fits::Fits::AUTO_CLOSE | fits::Fits::AUTO_CHECK);
fp.setHdu(hdu);
return readMetadata(fp, strip);
}

PTR(daf::base::PropertyList) readMetadata(fits::Fits & fitsfile, bool strip) {
auto metadata = std::make_shared<lsst::daf::base::PropertyList>();
fitsfile.readMetadata(*metadata, strip);
// if INHERIT=T, we want to also include header entries from the primary HDU
if (fitsfile.getHdu() != 1 && metadata->exists("INHERIT")) {
bool inherit = false;
if (metadata->typeOf("INHERIT") == typeid(std::string)) {
inherit = (metadata->get<std::string>("INHERIT") == "T");
} else {
inherit = metadata->get<bool>("INHERIT");
}
if (strip) metadata->remove("INHERIT");
if (inherit) {
fitsfile.setHdu(1);
// We don't want to just just call fitsfile.readMetadata to append the new keys,
// because PropertySet::get will return the last value added when multiple values
// are present and a scalar is requested; in that case, we want the non-inherited
// value to be added last, so it's the one that takes precedence.
PTR(daf::base::PropertyList) inherited(new daf::base::PropertyList);
fitsfile.readMetadata(*inherited, strip);
inherited->combine(metadata);
inherited.swap(metadata);
}
}
return metadata;
}


#define INSTANTIATE_KEY_OPS(r, data, T) \
template void Fits::updateKey(std::string const &, T const &, std::string const &); \
template void Fits::writeKey(std::string const &, T const &, std::string const &); \
Expand Down
3 changes: 2 additions & 1 deletion src/formatters/TanWcsFormatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ static char const* SVNid __attribute__((unused)) = "$Id$";
#include "lsst/afw/formatters/MaskedImageFormatter.h"
#include "lsst/afw/formatters/TanWcsFormatter.h"
#include "lsst/afw/image/TanWcs.h"
#include "lsst/afw/fits.h"

namespace {
LOG_LOGGER _log = LOG_GET("afw.TanWcsFormatter");
Expand Down Expand Up @@ -119,7 +120,7 @@ dafBase::Persistable* afwForm::TanWcsFormatter::read(
dafPersist::FitsStorage* fits = dynamic_cast<dafPersist::FitsStorage*>(storage.get());
int hdu = additionalData->get<int>("hdu", 0);
dafBase::PropertySet::Ptr md =
afwImg::readMetadata(fits->getPath(), hdu);
afw::fits::readMetadata(fits->getPath(), hdu);
afwImg::TanWcs* ip = new afwImg::TanWcs(md);
LOGL_DEBUG(_log, "TanWcsFormatter read end");
return ip;
Expand Down
3 changes: 2 additions & 1 deletion src/formatters/WcsFormatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ static char const* SVNid __attribute__((unused)) = "$Id$";
#include "lsst/afw/formatters/MaskedImageFormatter.h"
#include "lsst/afw/formatters/WcsFormatter.h"
#include "lsst/afw/image/Wcs.h"
#include "lsst/afw/fits.h"

namespace {
LOG_LOGGER _log = LOG_GET("afw.WcsFormatter");
Expand Down Expand Up @@ -121,7 +122,7 @@ dafBase::Persistable* afwForm::WcsFormatter::read(
dafPersist::FitsStorage* fits = dynamic_cast<dafPersist::FitsStorage*>(storage.get());
int hdu = additionalData->get<int>("hdu", 0);
dafBase::PropertySet::Ptr md =
afwImg::readMetadata(fits->getPath(), hdu);
afw::fits::readMetadata(fits->getPath(), hdu);
afwImg::Wcs* ip = new afwImg::Wcs(md);
LOGL_DEBUG(_log, "WcsFormatter read end");
return ip;
Expand Down
35 changes: 0 additions & 35 deletions src/image/Utils.cc

This file was deleted.