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.

Sign up for GitHub

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

Jump to bottom

DM-15767: reduce usage of daf.base.Persistable #44

Merged
merged 2 commits into from Oct 19, 2018
Merged

DM-15767: reduce usage of daf.base.Persistable #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6077318
Remove PropertySet/PropertyList inheritance from Persistable.
TallJimbo Sep 14, 2018
2e7874b
Modernize signatures and destructors.
TallJimbo Sep 14, 2018
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
19 changes: 7 additions & 12 deletions include/lsst/daf/base/PropertyList.h
View file
Open in desktop
Expand Up @@ -58,11 +58,6 @@

namespace lsst {
namespace daf {

namespace persistence {
class PropertyListFormatter;
} // namespace persistence

namespace base {

#if defined(__ICC)
Expand All @@ -77,9 +72,10 @@ class LSST_EXPORT PropertyList : public PropertySet {
typedef std::shared_ptr<PropertyList const> ConstPtr;

/// Construct an empty PropertyList
PropertyList(void);
PropertyList();

/// Destructor
virtual ~PropertyList(void);
virtual ~PropertyList() noexcept;

// Accessors

Expand All @@ -88,7 +84,7 @@ class LSST_EXPORT PropertyList : public PropertySet {
*
* @return PropertyList::Ptr pointing to the new copy.
*/
virtual PropertySet::Ptr deepCopy(void) const;
virtual PropertySet::Ptr deepCopy() const;

// I can't make copydoc work for this so...
/**
Expand Down Expand Up @@ -135,13 +131,13 @@ class LSST_EXPORT PropertyList : public PropertySet {
std::string const& getComment(std::string const& name) const;

/// Get the list of property names, in the order they were added
std::vector<std::string> getOrderedNames(void) const;
std::vector<std::string> getOrderedNames() const;

/// Begin iterator over the list of property names, in the order they were added
std::list<std::string>::const_iterator begin(void) const;
std::list<std::string>::const_iterator begin() const;

/// End iterator over the list of property names, in the order they were added
std::list<std::string>::const_iterator end(void) const;
std::list<std::string>::const_iterator end() const;

/// @copydoc PropertySet::toString()
virtual std::string toString(bool topLevelOnly = false, std::string const& indent = "") const;
Expand Down Expand Up @@ -288,7 +284,6 @@ class LSST_EXPORT PropertyList : public PropertySet {
virtual void remove(std::string const& name);

private:
LSST_PERSIST_FORMATTER(lsst::daf::persistence::PropertyListFormatter)

typedef std::unordered_map<std::string, std::string> CommentMap;

Expand Down
12 changes: 3 additions & 9 deletions include/lsst/daf/base/PropertySet.h
View file
Open in desktop
Expand Up @@ -58,19 +58,14 @@

namespace lsst {
namespace daf {

namespace persistence {
class PropertySetFormatter;
} // namespace persistence

namespace base {

#if defined(__ICC)
#pragma warning(push)
#pragma warning(disable : 444)
#endif

class LSST_EXPORT PropertySet : public Persistable, public Citizen {
class LSST_EXPORT PropertySet : public Citizen {
Copy link

Choose a reason for hiding this comment

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

should you remove #include "lsst/daf/base/Persistable.h"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, no - it's still used in some accessors (a PropertySet is no longer a Persistable itself, but it can still store Persistables and probably will need to until we can drop pex_policy completely).

public:
// Typedefs
typedef std::shared_ptr<PropertySet> Ptr;
Expand All @@ -84,7 +79,7 @@ class LSST_EXPORT PropertySet : public Persistable, public Citizen {
explicit PropertySet(bool flat = false);

/// Destructor
virtual ~PropertySet(void);
virtual ~PropertySet() noexcept;

// No copying
PropertySet(const PropertySet&) = delete;
Expand All @@ -101,7 +96,7 @@ class LSST_EXPORT PropertySet : public Persistable, public Citizen {
*
* @return PropertySet::Ptr pointing to the new copy.
*/
virtual Ptr deepCopy(void) const;
virtual Ptr deepCopy() const;

/**
* Get the number of names in the PropertySet, optionally including those in subproperties.
Expand Down Expand Up @@ -455,7 +450,6 @@ class LSST_EXPORT PropertySet : public Persistable, public Citizen {
virtual std::string _format(std::string const& name) const;

private:
LSST_PERSIST_FORMATTER(lsst::daf::persistence::PropertySetFormatter)

typedef std::unordered_map<std::string, std::shared_ptr<std::vector<boost::any> > > AnyMap;

Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/base/propertyContainer/propertySet.cc
View file
Open in desktop
Expand Up @@ -58,7 +58,7 @@ PYBIND11_MODULE(propertySet, mod) {
.def("name", &std::type_info::name)
.def("__hash__", &std::type_info::hash_code);

py::class_<PropertySet, std::shared_ptr<PropertySet>, Persistable, Citizen> cls(mod, "PropertySet");
py::class_<PropertySet, std::shared_ptr<PropertySet>, Citizen> cls(mod, "PropertySet");

cls.def(py::init<bool>(), "flat"_a = false);

Expand Down
12 changes: 6 additions & 6 deletions src/PropertyList.cc
View file
Open in desktop
Expand Up @@ -37,17 +37,17 @@ namespace base {

/** Constructor.
*/
PropertyList::PropertyList(void) : PropertySet(true) {}
PropertyList::PropertyList() : PropertySet(true) {}

/** Destructor.
*/
PropertyList::~PropertyList(void) {}
PropertyList::~PropertyList() noexcept = default;

///////////////////////////////////////////////////////////////////////////////
// Accessors
///////////////////////////////////////////////////////////////////////////////

PropertySet::Ptr PropertyList::deepCopy(void) const {
PropertySet::Ptr PropertyList::deepCopy() const {
Ptr n(new PropertyList);
n->PropertySet::combine(this->PropertySet::deepCopy());
n->_order = _order;
Expand Down Expand Up @@ -78,17 +78,17 @@ std::string const& PropertyList::getComment(std::string const& name) const {
return _comments.find(name)->second;
}

std::vector<std::string> PropertyList::getOrderedNames(void) const {
std::vector<std::string> PropertyList::getOrderedNames() const {
std::vector<std::string> v;
for (auto const& name : _order) {
v.push_back(name);
}
return v;
}

std::list<std::string>::const_iterator PropertyList::begin(void) const { return _order.begin(); }
std::list<std::string>::const_iterator PropertyList::begin() const { return _order.begin(); }

std::list<std::string>::const_iterator PropertyList::end(void) const { return _order.end(); }
std::list<std::string>::const_iterator PropertyList::end() const { return _order.end(); }

std::string PropertyList::toString(bool topLevelOnly, std::string const& indent) const {
std::ostringstream s;
Expand Down
4 changes: 2 additions & 2 deletions src/PropertySet.cc
View file
Open in desktop
Expand Up @@ -56,13 +56,13 @@ void _append(std::vector<boost::any>& dest, std::vector<T> const& src) {

PropertySet::PropertySet(bool flat) : Citizen(typeid(*this)), _flat(flat) {}

PropertySet::~PropertySet(void) {}
PropertySet::~PropertySet() noexcept = default;

///////////////////////////////////////////////////////////////////////////////
// Accessors
///////////////////////////////////////////////////////////////////////////////

PropertySet::Ptr PropertySet::deepCopy(void) const {
PropertySet::Ptr PropertySet::deepCopy() const {
Ptr n(new PropertySet(_flat));
for (auto const& elt : _map) {
if (elt.second->back().type() == typeid(Ptr)) {
Expand Down
2 changes: 0 additions & 2 deletions tests/test_PropertyList.cc
View file
Open in desktop
Expand Up @@ -56,8 +56,6 @@ BOOST_AUTO_TEST_CASE(bases) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a LsstDm
dafBase::PropertyList::Ptr plp(new dafBase::PropertyList);
dafBase::PropertySet::Ptr psp = plp;
BOOST_CHECK_EQUAL(!plp, false);
std::shared_ptr<dafBase::Persistable> pp = plp;
BOOST_CHECK_EQUAL(!pp, false);
std::shared_ptr<dafBase::Citizen> cp = plp;
BOOST_CHECK_EQUAL(!cp, false);
}
Expand Down
3 changes: 0 additions & 3 deletions tests/test_PropertySet_1.cc
View file
Open in desktop
Expand Up @@ -52,9 +52,6 @@ BOOST_AUTO_TEST_CASE(construct) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a Ls
BOOST_AUTO_TEST_CASE(bases) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a LsstDm-5-25 LsstDm-4-6 "Boost test
harness macros" */
dafBase::PropertySet::Ptr psp(new dafBase::PropertySet);
std::shared_ptr<dafBase::Persistable> pp =
std::dynamic_pointer_cast<dafBase::Persistable, dafBase::PropertySet>(psp);
BOOST_CHECK_EQUAL(!pp, false);
std::shared_ptr<dafBase::Citizen> cp =
std::dynamic_pointer_cast<dafBase::Citizen, dafBase::PropertySet>(psp);
BOOST_CHECK_EQUAL(!cp, false);
Expand Down