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-10777: Create TransformBoundedField #24

Merged
merged 3 commits into from
Jul 10, 2017
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
24 changes: 20 additions & 4 deletions include/astshim/Object.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ class Object {
Object &operator=(Object const &) = delete;
Object &operator=(Object &&) = default;

/**
Return True if this and `rhs` are the equal

For two objects be equal, they both must have the same attributes and all contained objects
must be equal.
*/
bool operator==(Object const &rhs) const;

/**
Return True if this and `rhs` are not equal

See operator== for details
*/
bool operator!=(Object const &rhs) const { return !(*this == rhs); };

/**
Construct an @ref Object from a string, using astFromString
*/
Expand Down Expand Up @@ -202,16 +217,17 @@ class Object {
/**
Print a textual description the object to an ostream.

It is provided primarily as an aid to debugging
@param[in, out] os The stream to which to write the string representation.
@param[in] showComments Show comments?
*/
void show(std::ostream &os) const;
void show(std::ostream &os, bool showComments = true) const;

/**
Return a textual description the object as a string.

It is provided primarily as an aid to debugging
@param[in] showComments Show comments?
*/
std::string show() const;
std::string show(bool showComments = true) const;

/**
Has this attribute been explicitly set (and not subsequently cleared)?
Expand Down
5 changes: 4 additions & 1 deletion python/astshim/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ PYBIND11_PLUGIN(object) {

cls.def("__str__", &Object::getClassName);
cls.def("__repr__", [](Object const &self) { return "astshim." + self.getClassName(); });
cls.def("__eq__", &Object::operator==, py::is_operator());
cls.def("__ne__", &Object::operator!=, py::is_operator());

cls.def_property_readonly("className", &Object::getClassName);
cls.def_property("id", &Object::getID, &Object::setID);
Expand All @@ -56,7 +58,8 @@ PYBIND11_PLUGIN(object) {
cls.def("getRefCount", &Object::getRefCount);
cls.def("lock", &Object::lock, "wait"_a);
cls.def("same", &Object::same, "other"_a);
cls.def("show", (std::string(Object::*)() const) & Object::show);
// do not wrap the ostream version of show, since there is no obvious Python equivalent to ostream
cls.def("show", (std::string(Object::*)(bool) const) & Object::show, "showComments"_a = true);
cls.def("test", &Object::test, "attrib"_a);
cls.def("unlock", &Object::unlock, "report"_a = false);
// do not wrap getRawPtr, since it returns a bare AST pointer
Expand Down
15 changes: 11 additions & 4 deletions src/Object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* the GNU General Public License along with this program. If not,
* see <https://www.lsstcorp.org/LegalNotices/>.
*/
#include <algorithm>
#include <functional>
#include <ostream>
#include <sstream>
Expand Down Expand Up @@ -76,6 +77,12 @@ extern "C" void sinkToOstream(const char *text) {

} // anonymous namespace

bool Object::operator==(Object const &rhs) const {
auto thisStr = this->show(false);
auto rhsStr = rhs.show(false);
return rhsStr == thisStr;
}

std::shared_ptr<Object> Object::_basicFromAstObject(AstObject *rawObj) {
static std::unordered_map<std::string, std::function<std::shared_ptr<Object>(AstObject *)>>
ClassCasterMap = {
Expand Down Expand Up @@ -139,8 +146,8 @@ std::shared_ptr<Class> Object::fromAstObject(AstObject *rawObj, bool copy) {
return retObject;
}

void Object::show(std::ostream &os) const {
auto ch = astChannel(nullptr, sinkToOstream, "%s", "");
void Object::show(std::ostream &os, bool showComments) const {
auto ch = astChannel(nullptr, sinkToOstream, "%s", showComments ? "" : "Comment=0");

// Store a poiner to the ostream in the channel, as required by sinkToOstream
astPutChannelData(ch, &os);
Expand All @@ -149,9 +156,9 @@ void Object::show(std::ostream &os) const {
assertOK();
}

std::string Object::show() const {
std::string Object::show(bool showComments) const {
std::ostringstream os;
show(os);
show(os, showComments);
return os.str();
}

Expand Down
54 changes: 54 additions & 0 deletions tests/test_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,38 @@ def test_error_handling(self):
except RuntimeError as e:
self.assertEqual(e.args[0].count("Error"), 1)

def test_equality(self):
"""Test __eq__ and __ne__
"""
frame = astshim.Frame(2)
zoomMap = astshim.ZoomMap(2, 1.5)
frameSet1 = astshim.FrameSet(frame, zoomMap, frame)
frameSet2 = astshim.FrameSet(frame, zoomMap, frame)
self.assertTrue(frameSet1 == frameSet2)
self.assertFalse(frameSet1 != frameSet2)
self.assertEqual(frameSet1, frameSet2)

# the base attribute of frameSet1 is not set; set the base attribute
# of framesSet2 and make sure the frame sets are now not equal
self.assertFalse(frameSet1.test("Base"))
frameSet2.base = 1
self.assertTrue(frameSet2.test("Base"))
self.assertFalse(frameSet1 == frameSet2)
self.assertTrue(frameSet1 != frameSet2)
self.assertNotEqual(frameSet1, frameSet2)

# make sure base is unset in the inverse of the inverse of frameSet1,
# else the equality test will fail for hard-to-understand reasons
self.assertFalse(frameSet1.getInverse().getInverse().test("Base"))
self.assertNotEqual(frameSet1, frameSet1.getInverse())
self.assertEqual(frameSet1, frameSet1.getInverse().getInverse())
self.assertFalse(frameSet1.getInverse().getInverse().test("Base"))

frame3 = astshim.Frame(2)
frame3.title = "Frame 3"
frameSet3 = astshim.FrameSet(frame3)
self.assertNotEqual(frameSet1, frameSet3)
Copy link

Choose a reason for hiding this comment

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

You test for the complicated cases, but not the easy ones (where there is no difference between unset and default value). Say: frameSet1 == frameSet1 and astshim.Frame(2) == astshim.Frame(2). Unlikely to fail, but still.

Copy link

Choose a reason for hiding this comment

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

Additionally you may want a test for a more complicated case where multiple attributes are present.

Copy link

Choose a reason for hiding this comment

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

And what happens if say title = "bla#with this? E.g. one of the things in the printed representation has a hash? I don't know if that can happen, but it may be a problem with the string comparison.

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 added a test for two objects with equal attributes. Good suggestion!

I don't think testing frameSet1 against itself is interesting because the fallback is pointer equality.

The attribute containing a hash is a good point, but no longer relevant because I am no longer stripping hash marks.


def test_id(self):
"""Test that ID is *not* transferred to copies"""
obj = astshim.ZoomMap(2, 1.3)
Expand All @@ -113,6 +145,28 @@ def test_ident(self):
cp = obj.copy()
self.assertEqual(cp.ident, "initial_ident")

def test_show(self):
obj = astshim.ZoomMap(2, 1.3)
desShowLines = [
" Begin ZoomMap \t# Zoom about the origin",
" Nin = 2 \t# Number of input coordinates",
" IsA Mapping \t# Mapping between coordinate systems",
" Zoom = 1.3 \t# Zoom factor",
" End ZoomMap",
"",
]
desShowLinesNoComments = [
" Begin ZoomMap",
" Nin = 2",
" IsA Mapping",
" Zoom = 1.3",
" End ZoomMap",
"",
]
self.assertEqual(obj.show(), "\n".join(desShowLines))
self.assertEqual(obj.show(True), "\n".join(desShowLines))
self.assertEqual(obj.show(False), "\n".join(desShowLinesNoComments))


if __name__ == "__main__":
unittest.main()
2 changes: 1 addition & 1 deletion tests/test_polyMap.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_PolyMapIterativeInverse(self):
indata_roundtrip = pm.applyInverse(outdata)
npt.assert_allclose(indata, indata_roundtrip, atol=1.0e-4)

def test_polyMapAtributes(self):
def test_polyMapAttributes(self):
coeff_f = np.array([
[1.2, 1, 2, 0],
[-0.5, 1, 1, 1],
Expand Down