DM-13189: add FunctorKey for Boxes #306
Conversation
| @@ -40,6 +40,11 @@ class PointKey(with_metaclass(TemplateMeta, object)): | |||
| PointKey.register((np.float64, 2), aggregates.Point2DKey) | |||
|
|
|||
|
|
|||
| # Because Boxes aren't templates themselves, we don't expose the fact that | |||
There was a problem hiding this comment.
Should this be in a docstring?
There was a problem hiding this comment.
I don't think so; the potential confusion this comment is trying to avoid is from seeing a change in the pattern established in the rest of the file, and hence I don't think there's much to explain for someone who isn't reading the code.
include/lsst/afw/table/aggregates.h
Outdated
| /** | ||
| * A FunctorKey used to get or set a geom::Box2I or Box2D from a (min, max) pair of PointKeys. | ||
| * | ||
| * the Box2IKey and Box2DKey typedefs should be preferred to using the template name directly. |
include/lsst/afw/table/aggregates.h
Outdated
| * | ||
| * @param[in,out] schema Schema to add fields to. | ||
| * @param[in] name Name prefix for all fields; suffixes above will be appended to this | ||
| * to form the full field names. |
There was a problem hiding this comment.
Please give an example, e.g. "somename_min_x"
| template <typename Box> | ||
| class BoxKey : public FunctorKey<Box> { | ||
| public: | ||
|
|
There was a problem hiding this comment.
Specify default/delete copy/move (RFC-209)?
| @@ -112,6 +112,52 @@ def testPointKey(self): | |||
| self.doTestPointKey("D", lsst.afw.table.Point2DKey, | |||
| lsst.afw.geom.Point2D) | |||
|
|
|||
| def doTestBoxKey(self, pointFieldType, functorKeyType, valueType): | |||
There was a problem hiding this comment.
Docstring, please, at least for the args.
tests/test_functorKeys.py
Outdated
| minKey = pointFieldType(schema["a_min"]) | ||
| maxKey = pointFieldType(schema["a_max"]) | ||
| # we create two equivalent functor keys, using the two different | ||
| # constructors |
There was a problem hiding this comment.
don't have to do 80 chars, I don't think...
There was a problem hiding this comment.
Correct, this is just what my editor does when I tell it to wrap (since it treats comments like docs, which do have to by 80 chars, I think). Anyhow, I agree it looks funny here; will fix.
|
Also, thanks for the reminder on RFC-209; I just had another reminder about that from another angle (DM-9932), and the implementation of the RFC is now in review. |
TallJimbo
force-pushed
the
tickets/DM-13189
branch
2 times, most recently
from
4d0cafd
to
607a951
Compare
TallJimbo
force-pushed
the
tickets/DM-13189
branch
from
607a951
to
404a483
Compare
This adds
FunctorKeysforBox2IandBox2D, which should start us towards recording boxes in tables in a consistent way.I've also updated three other classes in afw to use the new
FunctorKeybecause they were already using the same convention and hence the change could be done in a backwards-compatible way. Two others (AmpInfoRecordandChebyshev1Function2d) use a different convention and cannot be converted without changing APIs and/or persistence formats.