Skip to content

Commit

Permalink
Move BaseRecord.__str__ implementation to C++.
Browse files Browse the repository at this point in the history
Old implementation was very inefficient, even in Python, but
we can make it even faster by moving it to C++.
  • Loading branch information
TallJimbo committed Mar 15, 2018
1 parent 2e4184d commit a302b06
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 4 deletions.
9 changes: 9 additions & 0 deletions include/lsst/afw/table/BaseRecord.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#ifndef AFW_TABLE_BaseRecord_h_INCLUDED
#define AFW_TABLE_BaseRecord_h_INCLUDED

#include <iosfwd>

#include "lsst/base.h"
#include "lsst/afw/table/fwd.h"
#include "lsst/afw/table/Schema.h"
Expand Down Expand Up @@ -173,10 +175,17 @@ class BaseRecord : public daf::base::Citizen {

virtual ~BaseRecord() { _table->_destroy(*this); }

/// Write the record's content out, one field on each line.
friend std::ostream & operator<<(std::ostream & os, BaseRecord const & record);

protected:
/// Called by assign() after transferring fields to allow subclass data members to be copied.
virtual void _assign(BaseRecord const& other) {}

/// Called by operator<<. Overrides should call the base class implementation and append
/// additional fields on new lines, with the syntax "%(name)s: %(value)s".
virtual void _stream(std::ostream & os) const;

/// Construct a record with uninitialized data.
BaseRecord(std::shared_ptr<BaseTable> const& table) : daf::base::Citizen(typeid(this)), _table(table) {
table->_initialize(*this);
Expand Down
2 changes: 2 additions & 0 deletions python/lsst/afw/table/base/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ PyBaseRecord declareBaseRecord(py::module &mod) {
key.attr("set")(self, value);
};

utils::python::addOutputOp(cls, "__str__"); // __repr__ is defined in baseContinued.py

// The distinction between get/set and operator[] is meaningful in C++, because "record[k] = v"
// operates by returning an object that can be assigned to.
// But there's no meaningful difference between get/set and __getitem__/__setitem__.
Expand Down
4 changes: 0 additions & 4 deletions python/lsst/afw/table/base/baseContinued.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ def extract(self, *patterns, **kwds):
d[name] = self.get(schemaItem.key)
return d

def __str__(self):
return '\n'.join("%s: %s"%(x.field.getName(), self.extract("*")[x.field.getName()])
for x in self.schema)

def __repr__(self):
return "%s\n%s" % (type(self), str(self))

Expand Down
17 changes: 17 additions & 0 deletions src/table/BaseRecord.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// -*- lsst-c++ -*-

#include <cstring>
#include <iostream>

#include "lsst/pex/exceptions.h"
#include "lsst/afw/table/BaseRecord.h"
Expand Down Expand Up @@ -90,6 +91,22 @@ void BaseRecord::assign(BaseRecord const& other, SchemaMapper const& mapper) {
mapper.forEach(CopyValue(&other, this)); // use the functor we defined above
this->_assign(other); // let derived classes assign their own stuff
}


void BaseRecord::_stream(std::ostream & os) const {
getSchema().forEach(
[this, &os](auto const & item) {
os << item.field.getName() << ": " << this->get(item.key) << std::endl;
}
);
}

std::ostream & operator<<(std::ostream & os, BaseRecord const & record) {
record._stream(os);
return os;
}


}
}
} // namespace lsst::afw::table

0 comments on commit a302b06

Please sign in to comment.