From fa747c141af9674f0503987457feb54b234c4aa3 Mon Sep 17 00:00:00 2001 From: Pierre-Francois Leget Date: Wed, 19 Mar 2025 14:54:26 -0700 Subject: [PATCH 1/5] Add access to color value. --- include/lsst/afw/image/Color.h | 2 +- python/lsst/afw/image/_color.cc | 1 + tests/test_color.py | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/lsst/afw/image/Color.h b/include/lsst/afw/image/Color.h index 19d44d880b..d44cd5ad8c 100644 --- a/include/lsst/afw/image/Color.h +++ b/include/lsst/afw/image/Color.h @@ -34,6 +34,7 @@ class Color final { /// Whether the color is the special value that indicates that it is unspecified. bool isIndeterminate() const noexcept { return std::isnan(_g_r); } + double getColorValue() const noexcept { return _g_r;} //@{ /** @@ -55,7 +56,6 @@ class Color final { /// Return a hash of this object. std::size_t hash_value() const noexcept { return isIndeterminate() ? 42 : std::hash()(_g_r); } -private: double _g_r; }; } // namespace image diff --git a/python/lsst/afw/image/_color.cc b/python/lsst/afw/image/_color.cc index 8097be1ce7..9863bbbf80 100644 --- a/python/lsst/afw/image/_color.cc +++ b/python/lsst/afw/image/_color.cc @@ -53,6 +53,7 @@ void wrapColor(lsst::cpputils::python::WrapperCollection &wrappers) { /* Members */ cls.def("isIndeterminate", &Color::isIndeterminate); + cls.def("getColorValue", &Color::getColorValue); }); } diff --git a/tests/test_color.py b/tests/test_color.py index 90cb197f36..9c3ac7b514 100644 --- a/tests/test_color.py +++ b/tests/test_color.py @@ -35,6 +35,10 @@ def testIsIndeterminate(self): self.assertTrue(afwImage.Color().isIndeterminate()) self.assertFalse(afwImage.Color(1.2).isIndeterminate()) + def testGetColor(self): + color = afwImage.Color(0.42) + self.assertEqual(color.getColorValue(), 0.42) + class MemoryTester(lsst.utils.tests.MemoryTestCase): pass From 1e43080e53e7c56c011ef23be218f9d738f4740e Mon Sep 17 00:00:00 2001 From: Pierre-Francois Leget Date: Tue, 6 May 2025 12:56:28 -0700 Subject: [PATCH 2/5] Add color type. --- include/lsst/afw/image/Color.h | 66 ++++++++++++++++++--------------- python/lsst/afw/image/_color.cc | 42 ++++++++++++--------- tests/test_color.py | 6 +-- tests/test_imageHash.cc | 10 ----- 4 files changed, 64 insertions(+), 60 deletions(-) diff --git a/include/lsst/afw/image/Color.h b/include/lsst/afw/image/Color.h index d44cd5ad8c..9f0fdd20d3 100644 --- a/include/lsst/afw/image/Color.h +++ b/include/lsst/afw/image/Color.h @@ -8,11 +8,14 @@ #include #include +#include +#include namespace lsst { namespace afw { namespace image { +// TO DO: Change description here. /** * Describe the colour of a source * @@ -24,7 +27,15 @@ namespace image { */ class Color final { public: - explicit Color(double g_r = std::numeric_limits::quiet_NaN()) : _g_r(g_r) {} + /// Default: indeterminate color + Color() noexcept + : _value(std::numeric_limits::quiet_NaN()), + _type(), + _indeterminate(true) {} + + /// Fully-specified color: both a numeric value and its type string + Color(double value, std::string const & type) noexcept + : _value(value), _type(type), _indeterminate(false) {} Color(Color const &) = default; Color(Color &&) = default; @@ -32,43 +43,40 @@ class Color final { Color &operator=(Color &&) = default; ~Color() noexcept = default; - /// Whether the color is the special value that indicates that it is unspecified. - bool isIndeterminate() const noexcept { return std::isnan(_g_r); } - double getColorValue() const noexcept { return _g_r;} + /// Whether this Color was default‑constructed (i.e. has no value/type). + bool isIndeterminate() const noexcept { return _indeterminate; } + + /// The numeric color value; only valid if !isIndeterminate(). + double getColorValue() const noexcept { return _value; } + + /// The color type string (e.g. "g-r"); only valid if !isIndeterminate(). + std::string const & getColorType() const noexcept { return _type; } - //@{ /** - * Equality comparison for colors - * - * Just a placeholder like everything else, but we explicitly let indeterminate colors compare - * as equal. + * Equality comparison for colors. * - * In the future, we'll probably want some way of doing fuzzy comparisons on colors, but then - * we'd have to define some kind of "color difference" matric, and it's not worthwhile doing - * that yet. + * Indeterminate colors compare equal to each other; fully-specified + * colors compare by both value and type. */ - bool operator==(Color const &other) const noexcept { - return (isIndeterminate() && other.isIndeterminate()) || other._g_r == _g_r; + bool operator==(Color const & other) const noexcept { + if (_indeterminate && other._indeterminate) { + return true; + } + if (_indeterminate != other._indeterminate) { + return false; + } + return _value == other._value && _type == other._type; } - bool operator!=(Color const &other) const noexcept { return !operator==(other); } - //@} + bool operator!=(Color const & other) const noexcept { return !(*this == other); } - /// Return a hash of this object. - std::size_t hash_value() const noexcept { return isIndeterminate() ? 42 : std::hash()(_g_r); } - - double _g_r; +private: + double _value; + std::string _type; + bool _indeterminate; }; + } // namespace image } // namespace afw } // namespace lsst -namespace std { -template <> -struct hash { - using argument_type = lsst::afw::image::Color; - using result_type = size_t; - size_t operator()(argument_type const &obj) const noexcept { return obj.hash_value(); } -}; -} // namespace std - #endif diff --git a/python/lsst/afw/image/_color.cc b/python/lsst/afw/image/_color.cc index 9863bbbf80..bf8abfd3f9 100644 --- a/python/lsst/afw/image/_color.cc +++ b/python/lsst/afw/image/_color.cc @@ -25,6 +25,7 @@ #include "lsst/cpputils/python.h" #include +#include #include "lsst/afw/image/Color.h" @@ -37,24 +38,29 @@ namespace image { using PyColor = py::class_>; -void wrapColor(lsst::cpputils::python::WrapperCollection &wrappers) { - /* Module level */ - wrappers.wrapType(PyColor(wrappers.module, "Color"), [](auto &mod, auto &cls) { - /* Constructors */ - cls.def(py::init(), "g_r"_a = std::numeric_limits::quiet_NaN()); - - /* Operators */ - cls.def( - "__eq__", [](Color const &self, Color const &other) { return self == other; }, - py::is_operator()); - cls.def( - "__ne__", [](Color const &self, Color const &other) { return self != other; }, - py::is_operator()); - - /* Members */ - cls.def("isIndeterminate", &Color::isIndeterminate); - cls.def("getColorValue", &Color::getColorValue); - }); +void wrapColor(lsst::cpputils::python::WrapperCollection & wrappers) { + PyColor cls(wrappers.module, "Color"); + + /* Constructors */ + cls + // default ctor → indeterminate color + .def(py::init<>()) + // fully-specified ctor: both ColorValue and ColorType required + .def(py::init(), + "ColorValue"_a, "ColorType"_a); + + /* Operators */ + cls.def( + "__eq__", [](Color const & self, Color const & other) { return self == other; }, + py::is_operator()); + cls.def( + "__ne__", [](Color const & self, Color const & other) { return self != other; }, + py::is_operator()); + + /* Members */ + cls.def("isIndeterminate", &Color::isIndeterminate); + cls.def("getColorValue", &Color::getColorValue); + cls.def("getColorType", &Color::getColorType); } } // namespace image diff --git a/tests/test_color.py b/tests/test_color.py index 9c3ac7b514..6dfaba370a 100644 --- a/tests/test_color.py +++ b/tests/test_color.py @@ -28,15 +28,15 @@ class ColorTestCase(lsst.utils.tests.TestCase): def testCtor(self): afwImage.Color() - afwImage.Color(1.2) + afwImage.Color(ColorValue=1.2, ColorType="g-r") def testIsIndeterminate(self): """Test that a default-constructed Color tests True, but ones with a g-r value test False""" self.assertTrue(afwImage.Color().isIndeterminate()) - self.assertFalse(afwImage.Color(1.2).isIndeterminate()) + self.assertFalse(afwImage.Color(ColorValue=1.2, ColorType="g-r").isIndeterminate()) def testGetColor(self): - color = afwImage.Color(0.42) + color = afwImage.Color(ColorValue=0.42, ColorType="g-r") self.assertEqual(color.getColorValue(), 0.42) diff --git a/tests/test_imageHash.cc b/tests/test_imageHash.cc index 0471057935..00728aaa7e 100644 --- a/tests/test_imageHash.cc +++ b/tests/test_imageHash.cc @@ -31,21 +31,11 @@ #include "lsst/cpputils/tests.h" #include "lsst/afw/image.h" -#include "lsst/afw/image/Color.h" namespace lsst { namespace afw { namespace image { -BOOST_AUTO_TEST_CASE(ColorHash) { - cpputils::assertValidHash(); - - cpputils::assertHashesEqual(Color(), Color()); - cpputils::assertHashesEqual(Color(std::numeric_limits::quiet_NaN()), - Color(std::numeric_limits::signaling_NaN())); - cpputils::assertHashesEqual(Color(2.7), Color(2.7)); -} - BOOST_AUTO_TEST_CASE(PixelHash) { using IntPixel = pixel::Pixel; using FloatPixel = pixel::Pixel; From 61b66a3b0ec07227f38b5b02dfb0e4a1a9e26b2e Mon Sep 17 00:00:00 2001 From: Pierre-Francois Leget Date: Wed, 7 May 2025 10:51:55 -0700 Subject: [PATCH 3/5] revert and simplify --- include/lsst/afw/image/Color.h | 94 +++++++++++++++------------------ python/lsst/afw/image/_color.cc | 85 ++++++++++++++--------------- tests/test_color.py | 9 ++-- 3 files changed, 90 insertions(+), 98 deletions(-) diff --git a/include/lsst/afw/image/Color.h b/include/lsst/afw/image/Color.h index 9f0fdd20d3..1eb6aa3840 100644 --- a/include/lsst/afw/image/Color.h +++ b/include/lsst/afw/image/Color.h @@ -1,41 +1,31 @@ // -*- lsst-c++ -*- /* - * Capture the colour of an object - */ +* Capture the colour of an object +*/ #ifndef LSST_AFW_IMAGE_COLOR_H #define LSST_AFW_IMAGE_COLOR_H #include -#include #include -#include +#include namespace lsst { namespace afw { namespace image { -// TO DO: Change description here. /** - * Describe the colour of a source - * - * We need a concept of colour more general than "g - r" in order to calculate e.g. atmospheric dispersion - * or a source's PSF - * - * @note This is very much just a place holder until we work out what we need. A full SED may be required, - * in which case a constructor from an SED name might be appropriate, or a couple of colours, or ... - */ +* Describe the colour of a source +* +* We need a concept of colour more general than "g - r" in order to calculate e.g. atmospheric dispersion +* or a source's PSF +* +* @note This is very much just a place holder until we work out what we need. A full SED may be required, +* in which case a constructor from an SED name might be appropriate, or a couple of colours, or ... +*/ class Color final { public: - /// Default: indeterminate color - Color() noexcept - : _value(std::numeric_limits::quiet_NaN()), - _type(), - _indeterminate(true) {} - - /// Fully-specified color: both a numeric value and its type string - Color(double value, std::string const & type) noexcept - : _value(value), _type(type), _indeterminate(false) {} + explicit Color(double g_r = std::numeric_limits::quiet_NaN(), std::string color_type = "") : _g_r(g_r), _color_type(std::move(color_type)) {} Color(Color const &) = default; Color(Color &&) = default; @@ -43,40 +33,44 @@ class Color final { Color &operator=(Color &&) = default; ~Color() noexcept = default; - /// Whether this Color was default‑constructed (i.e. has no value/type). - bool isIndeterminate() const noexcept { return _indeterminate; } - - /// The numeric color value; only valid if !isIndeterminate(). - double getColorValue() const noexcept { return _value; } - - /// The color type string (e.g. "g-r"); only valid if !isIndeterminate(). - std::string const & getColorType() const noexcept { return _type; } + /// Whether the color is the special value that indicates that it is unspecified. + bool isIndeterminate() const noexcept { return std::isnan(_g_r); } + double getColorValue() const noexcept { return _g_r;} + //@{ /** - * Equality comparison for colors. - * - * Indeterminate colors compare equal to each other; fully-specified - * colors compare by both value and type. - */ - bool operator==(Color const & other) const noexcept { - if (_indeterminate && other._indeterminate) { - return true; - } - if (_indeterminate != other._indeterminate) { - return false; - } - return _value == other._value && _type == other._type; + * Equality comparison for colors + * + * Just a placeholder like everything else, but we explicitly let indeterminate colors compare + * as equal. + * + * In the future, we'll probably want some way of doing fuzzy comparisons on colors, but then + * we'd have to define some kind of "color difference" matric, and it's not worthwhile doing + * that yet. + */ + bool operator==(Color const &other) const noexcept { + return (isIndeterminate() && other.isIndeterminate()) || other._g_r == _g_r; } - bool operator!=(Color const & other) const noexcept { return !(*this == other); } + bool operator!=(Color const &other) const noexcept { return !operator==(other); } + //@} -private: - double _value; - std::string _type; - bool _indeterminate; -}; + /// Return a hash of this object. + std::size_t hash_value() const noexcept { return isIndeterminate() ? 42 : std::hash()(_g_r); } + double _g_r; + std::string _color_type; +}; } // namespace image } // namespace afw } // namespace lsst -#endif +namespace std { +template <> +struct hash { + using argument_type = lsst::afw::image::Color; + using result_type = size_t; + size_t operator()(argument_type const &obj) const noexcept { return obj.hash_value(); } +}; +} // namespace std + +#endif \ No newline at end of file diff --git a/python/lsst/afw/image/_color.cc b/python/lsst/afw/image/_color.cc index bf8abfd3f9..a0fdf95acd 100644 --- a/python/lsst/afw/image/_color.cc +++ b/python/lsst/afw/image/_color.cc @@ -1,25 +1,25 @@ /* - * This file is part of afw. - * - * Developed for the LSST Data Management System. - * This product includes software developed by the LSST Project - * (https://www.lsst.org). - * See the COPYRIGHT file at the top-level directory of this distribution - * for details of code ownership. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ +* This file is part of afw. +* +* Developed for the LSST Data Management System. +* This product includes software developed by the LSST Project +* (https://www.lsst.org). +* See the COPYRIGHT file at the top-level directory of this distribution +* for details of code ownership. +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ #include "pybind11/pybind11.h" #include "lsst/cpputils/python.h" @@ -38,31 +38,26 @@ namespace image { using PyColor = py::class_>; -void wrapColor(lsst::cpputils::python::WrapperCollection & wrappers) { - PyColor cls(wrappers.module, "Color"); - - /* Constructors */ - cls - // default ctor → indeterminate color - .def(py::init<>()) - // fully-specified ctor: both ColorValue and ColorType required - .def(py::init(), - "ColorValue"_a, "ColorType"_a); - - /* Operators */ - cls.def( - "__eq__", [](Color const & self, Color const & other) { return self == other; }, - py::is_operator()); - cls.def( - "__ne__", [](Color const & self, Color const & other) { return self != other; }, - py::is_operator()); - - /* Members */ - cls.def("isIndeterminate", &Color::isIndeterminate); - cls.def("getColorValue", &Color::getColorValue); - cls.def("getColorType", &Color::getColorType); +void wrapColor(lsst::cpputils::python::WrapperCollection &wrappers) { + /* Module level */ + wrappers.wrapType(PyColor(wrappers.module, "Color"), [](auto &mod, auto &cls) { + /* Constructors */ + cls.def(py::init(), "g_r"_a = std::numeric_limits::quiet_NaN(), "color_type"_a = ""); + + /* Operators */ + cls.def( + "__eq__", [](Color const &self, Color const &other) { return self == other; }, + py::is_operator()); + cls.def( + "__ne__", [](Color const &self, Color const &other) { return self != other; }, + py::is_operator()); + + /* Members */ + cls.def("isIndeterminate", &Color::isIndeterminate); + cls.def("getColorValue", &Color::getColorValue); + }); } } // namespace image } // namespace afw -} // namespace lsst +} // namespace lsst \ No newline at end of file diff --git a/tests/test_color.py b/tests/test_color.py index 6dfaba370a..9952958f31 100644 --- a/tests/test_color.py +++ b/tests/test_color.py @@ -28,15 +28,18 @@ class ColorTestCase(lsst.utils.tests.TestCase): def testCtor(self): afwImage.Color() - afwImage.Color(ColorValue=1.2, ColorType="g-r") + afwImage.Color(1.2) + #afwImage.Color(ColorValue=1.2, ColorType="g-r") def testIsIndeterminate(self): """Test that a default-constructed Color tests True, but ones with a g-r value test False""" self.assertTrue(afwImage.Color().isIndeterminate()) - self.assertFalse(afwImage.Color(ColorValue=1.2, ColorType="g-r").isIndeterminate()) + self.assertFalse(afwImage.Color(1.2).isIndeterminate()) + #self.assertFalse(afwImage.Color(ColorValue=1.2, ColorType="g-r").isIndeterminate()) def testGetColor(self): - color = afwImage.Color(ColorValue=0.42, ColorType="g-r") + #color = afwImage.Color(ColorValue=0.42, ColorType="g-r") + color = afwImage.Color(0.42) self.assertEqual(color.getColorValue(), 0.42) From 951bb6069cfeb3c67768680ff04f0b3304a31ae9 Mon Sep 17 00:00:00 2001 From: Pierre-Francois Leget Date: Wed, 7 May 2025 10:56:41 -0700 Subject: [PATCH 4/5] typo --- include/lsst/afw/image/Color.h | 38 ++++++++++++++-------------- python/lsst/afw/image/_color.cc | 44 ++++++++++++++++----------------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/include/lsst/afw/image/Color.h b/include/lsst/afw/image/Color.h index 1eb6aa3840..a9483c5bd4 100644 --- a/include/lsst/afw/image/Color.h +++ b/include/lsst/afw/image/Color.h @@ -1,7 +1,7 @@ // -*- lsst-c++ -*- /* -* Capture the colour of an object -*/ + * Capture the colour of an object + */ #ifndef LSST_AFW_IMAGE_COLOR_H #define LSST_AFW_IMAGE_COLOR_H @@ -15,14 +15,14 @@ namespace afw { namespace image { /** -* Describe the colour of a source -* -* We need a concept of colour more general than "g - r" in order to calculate e.g. atmospheric dispersion -* or a source's PSF -* -* @note This is very much just a place holder until we work out what we need. A full SED may be required, -* in which case a constructor from an SED name might be appropriate, or a couple of colours, or ... -*/ + * Describe the colour of a source + * + * We need a concept of colour more general than "g - r" in order to calculate e.g. atmospheric dispersion + * or a source's PSF + * + * @note This is very much just a place holder until we work out what we need. A full SED may be required, + * in which case a constructor from an SED name might be appropriate, or a couple of colours, or ... + */ class Color final { public: explicit Color(double g_r = std::numeric_limits::quiet_NaN(), std::string color_type = "") : _g_r(g_r), _color_type(std::move(color_type)) {} @@ -39,15 +39,15 @@ class Color final { //@{ /** - * Equality comparison for colors - * - * Just a placeholder like everything else, but we explicitly let indeterminate colors compare - * as equal. - * - * In the future, we'll probably want some way of doing fuzzy comparisons on colors, but then - * we'd have to define some kind of "color difference" matric, and it's not worthwhile doing - * that yet. - */ + * Equality comparison for colors + * + * Just a placeholder like everything else, but we explicitly let indeterminate colors compare + * as equal. + * + * In the future, we'll probably want some way of doing fuzzy comparisons on colors, but then + * we'd have to define some kind of "color difference" matric, and it's not worthwhile doing + * that yet. + */ bool operator==(Color const &other) const noexcept { return (isIndeterminate() && other.isIndeterminate()) || other._g_r == _g_r; } diff --git a/python/lsst/afw/image/_color.cc b/python/lsst/afw/image/_color.cc index a0fdf95acd..750dacf184 100644 --- a/python/lsst/afw/image/_color.cc +++ b/python/lsst/afw/image/_color.cc @@ -1,25 +1,25 @@ /* -* This file is part of afw. -* -* Developed for the LSST Data Management System. -* This product includes software developed by the LSST Project -* (https://www.lsst.org). -* See the COPYRIGHT file at the top-level directory of this distribution -* for details of code ownership. -* -* This program is free software: you can redistribute it and/or modify -* it under the terms of the GNU General Public License as published by -* the Free Software Foundation, either version 3 of the License, or -* (at your option) any later version. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see . -*/ + * This file is part of afw. + * + * Developed for the LSST Data Management System. + * This product includes software developed by the LSST Project + * (https://www.lsst.org). + * See the COPYRIGHT file at the top-level directory of this distribution + * for details of code ownership. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ #include "pybind11/pybind11.h" #include "lsst/cpputils/python.h" @@ -60,4 +60,4 @@ void wrapColor(lsst::cpputils::python::WrapperCollection &wrappers) { } // namespace image } // namespace afw -} // namespace lsst \ No newline at end of file +} // namespace lsst From 1b359aa38905d2dc808ac31473c51419ab858193 Mon Sep 17 00:00:00 2001 From: Pierre-Francois Leget Date: Wed, 7 May 2025 13:53:45 -0700 Subject: [PATCH 5/5] it works... --- include/lsst/afw/image/Color.h | 65 ++++++++++++++++++--------------- python/lsst/afw/image/_color.cc | 43 ++++++++++++---------- tests/test_color.py | 10 ++--- 3 files changed, 63 insertions(+), 55 deletions(-) diff --git a/include/lsst/afw/image/Color.h b/include/lsst/afw/image/Color.h index a9483c5bd4..818d386f7b 100644 --- a/include/lsst/afw/image/Color.h +++ b/include/lsst/afw/image/Color.h @@ -7,13 +7,14 @@ #define LSST_AFW_IMAGE_COLOR_H #include -#include #include +#include namespace lsst { namespace afw { namespace image { +// TO DO: Change description here. /** * Describe the colour of a source * @@ -25,7 +26,15 @@ namespace image { */ class Color final { public: - explicit Color(double g_r = std::numeric_limits::quiet_NaN(), std::string color_type = "") : _g_r(g_r), _color_type(std::move(color_type)) {} + /// Default: indeterminate color + Color() noexcept + : _color_value(std::numeric_limits::quiet_NaN()), + _color_type(), + _indeterminate(true) {} + + /// Fully-specified color: both a numeric value and its type string + Color(double color_value, std::string const & color_type) noexcept + : _color_value(color_value), _color_type(color_type), _indeterminate(false) {} Color(Color const &) = default; Color(Color &&) = default; @@ -33,44 +42,40 @@ class Color final { Color &operator=(Color &&) = default; ~Color() noexcept = default; - /// Whether the color is the special value that indicates that it is unspecified. - bool isIndeterminate() const noexcept { return std::isnan(_g_r); } - double getColorValue() const noexcept { return _g_r;} + /// Whether this Color was default‑constructed (i.e. has no value/type). + bool isIndeterminate() const noexcept { return _indeterminate; } + + /// The numeric color value; only valid if !isIndeterminate(). + double getColorValue() const noexcept { return _color_value; } + + /// The color type string (e.g. "g-r"); only valid if !isIndeterminate(). + std::string const & getColorType() const noexcept { return _color_type; } - //@{ /** - * Equality comparison for colors + * Equality comparison for colors. * - * Just a placeholder like everything else, but we explicitly let indeterminate colors compare - * as equal. - * - * In the future, we'll probably want some way of doing fuzzy comparisons on colors, but then - * we'd have to define some kind of "color difference" matric, and it's not worthwhile doing - * that yet. + * Indeterminate colors compare equal to each other; fully-specified + * colors compare by both value and type. */ - bool operator==(Color const &other) const noexcept { - return (isIndeterminate() && other.isIndeterminate()) || other._g_r == _g_r; + bool operator==(Color const & other) const noexcept { + if (_indeterminate && other._indeterminate) { + return true; + } + if (_indeterminate != other._indeterminate) { + return false; + } + return _color_value == other._color_value && _color_type == other._color_type; } - bool operator!=(Color const &other) const noexcept { return !operator==(other); } - //@} - - /// Return a hash of this object. - std::size_t hash_value() const noexcept { return isIndeterminate() ? 42 : std::hash()(_g_r); } + bool operator!=(Color const & other) const noexcept { return !(*this == other); } - double _g_r; +private: + double _color_value; std::string _color_type; + bool _indeterminate; }; + } // namespace image } // namespace afw } // namespace lsst -namespace std { -template <> -struct hash { - using argument_type = lsst::afw::image::Color; - using result_type = size_t; - size_t operator()(argument_type const &obj) const noexcept { return obj.hash_value(); } -}; -} // namespace std - #endif \ No newline at end of file diff --git a/python/lsst/afw/image/_color.cc b/python/lsst/afw/image/_color.cc index 750dacf184..69a0166262 100644 --- a/python/lsst/afw/image/_color.cc +++ b/python/lsst/afw/image/_color.cc @@ -38,26 +38,31 @@ namespace image { using PyColor = py::class_>; -void wrapColor(lsst::cpputils::python::WrapperCollection &wrappers) { - /* Module level */ - wrappers.wrapType(PyColor(wrappers.module, "Color"), [](auto &mod, auto &cls) { - /* Constructors */ - cls.def(py::init(), "g_r"_a = std::numeric_limits::quiet_NaN(), "color_type"_a = ""); - - /* Operators */ - cls.def( - "__eq__", [](Color const &self, Color const &other) { return self == other; }, - py::is_operator()); - cls.def( - "__ne__", [](Color const &self, Color const &other) { return self != other; }, - py::is_operator()); - - /* Members */ - cls.def("isIndeterminate", &Color::isIndeterminate); - cls.def("getColorValue", &Color::getColorValue); - }); +void wrapColor(lsst::cpputils::python::WrapperCollection & wrappers) { + PyColor cls(wrappers.module, "Color"); + + /* Constructors */ + cls + // default ctor → indeterminate color + .def(py::init<>()) + // fully-specified ctor: both ColorValue and ColorType required + .def(py::init(), + "colorValue"_a, "colorType"_a); + + /* Operators */ + cls.def( + "__eq__", [](Color const & self, Color const & other) { return self == other; }, + py::is_operator()); + cls.def( + "__ne__", [](Color const & self, Color const & other) { return self != other; }, + py::is_operator()); + + /* Members */ + cls.def("isIndeterminate", &Color::isIndeterminate); + cls.def("getColorValue", &Color::getColorValue); + cls.def("getColorType", &Color::getColorType); } } // namespace image } // namespace afw -} // namespace lsst +} // namespace lsst \ No newline at end of file diff --git a/tests/test_color.py b/tests/test_color.py index 9952958f31..fbd2363c0b 100644 --- a/tests/test_color.py +++ b/tests/test_color.py @@ -28,19 +28,17 @@ class ColorTestCase(lsst.utils.tests.TestCase): def testCtor(self): afwImage.Color() - afwImage.Color(1.2) - #afwImage.Color(ColorValue=1.2, ColorType="g-r") + afwImage.Color(colorValue=1.2, colorType="g-r") def testIsIndeterminate(self): """Test that a default-constructed Color tests True, but ones with a g-r value test False""" self.assertTrue(afwImage.Color().isIndeterminate()) - self.assertFalse(afwImage.Color(1.2).isIndeterminate()) - #self.assertFalse(afwImage.Color(ColorValue=1.2, ColorType="g-r").isIndeterminate()) + self.assertFalse(afwImage.Color(colorValue=1.2, colorType="g-r").isIndeterminate()) def testGetColor(self): - #color = afwImage.Color(ColorValue=0.42, ColorType="g-r") - color = afwImage.Color(0.42) + color = afwImage.Color(colorValue=0.42, colorType="g-r") self.assertEqual(color.getColorValue(), 0.42) + self.assertEqual(color.getColorType(), "g-r") class MemoryTester(lsst.utils.tests.MemoryTestCase):