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-12025: make Transform pickleable #278
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very nice. Two minor requests.
python/lsst/afw/geom/testUtils.py
Outdated
self.assertEqual(transform1.getFrameSet(), transform2.getFrameSet()) | ||
|
||
fromEndpoint = transform1.fromEndpoint | ||
toEndpoint = transform1.toEndpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this is pre-existing code, but... for TransformGenericToGeneric
the types may match but the # of axes may differ. I worry that this could cause a confusing test failure where you are comparing the results of applying transforms (below).
Please either assert that the corresponding endpoints are equal for the two transforms (Endpoint does define an equality test, though only in Python), or assert that the # of inputs and outputs match for the two transforms.
@@ -82,6 +82,8 @@ PYBIND11_PLUGIN(_interpolate) { | |||
"x"_a, "y"_a, "style"_a = Interpolate::AKIMA_SPLINE); | |||
|
|||
mod.def("stringToInterpStyle", stringToInterpStyle, "style"_a); | |||
mod.def("lookupMaxInterpStyle", lookupMaxInterpStyle, "n"_a); | |||
mod.def("lookupMinInterpPoints", lookupMinInterpPoints, "style"_a); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be much appreciated if you could add a few simple unit tests for these (including stringToInterpStyle
).
a1235bf
to
7385f6d
Compare
The key functionality was already present (readString, writeString methods) and just needed to be hooked up to the pickle machinery and tested.
These functions were previously wrapped with SWIG, and it's useful to have them wrapped with pybind.
Add tests for lookupMaxInterpStyle, lookupMinInterpPoints, and stringToInterpStyle
Make `assertTransformsEqual` produce a clearer failure if two transforms have a corresponding endpoint that is generic and has a different number of axes.
7385f6d
to
254a6fb
Compare
The key functionality was already present (readString, writeString
methods) and just needed to be hooked up to the pickle machinery
and tested.