diff --git a/include/boost/histogram/axis.hpp b/include/boost/histogram/axis.hpp index d3165d3a..4105ae99 100644 --- a/include/boost/histogram/axis.hpp +++ b/include/boost/histogram/axis.hpp @@ -38,7 +38,7 @@ namespace detail { boost::string_ref value; }; - template + template struct real_bin { int idx; @@ -263,9 +263,6 @@ public: const_iterator end() const { return const_iterator(*this, uoflow() ? bins() + 1 : bins()); } - value_type left(int idx) const { return operator[](idx); } - value_type right(int idx) const { return operator[](idx+1); } - private: value_type min_ = 0.0, delta_ = 1.0; @@ -275,10 +272,9 @@ private: /** Axis for real-valued angles. * - * There are no overflow/underflow bins for this axis, - * since the axis is circular and wraps around after - * \f$2 \pi\f$. - * Binning is a O(1) operation. + * The axis is circular and wraps around reaching the + * perimeter value. Therefore, there are no overflow/underflow + * bins for this axis. Binning is a O(1) operation. */ template class circular_axis: public axis_base { @@ -337,9 +333,6 @@ public: const_iterator end() const { return const_iterator(*this, bins()); } - value_type left(int idx) const { return operator[](idx); } - value_type right(int idx) const { return operator[](idx+1); } - private: value_type phase_ = 0.0, perimeter_ = 1.0; @@ -446,9 +439,6 @@ public: const_iterator end() const { return const_iterator(*this, uoflow() ? bins() + 1 : bins()); } - value_type left(int idx) const { return operator[](idx); } - value_type right(int idx) const { return operator[](idx+1); } - private: std::unique_ptr x_; // smaller size compared to std::vector diff --git a/include/boost/histogram/axis_ostream_operators.hpp b/include/boost/histogram/axis_ostream_operators.hpp index fbc31e93..a31d14a7 100644 --- a/include/boost/histogram/axis_ostream_operators.hpp +++ b/include/boost/histogram/axis_ostream_operators.hpp @@ -20,7 +20,7 @@ template inline std::ostream& operator<<(std::ostream& os, const regular_axis& a) { os << "regular_axis(" << a.bins() << ", " << a[0] << ", " << a[a.bins()]; - if (!detail::empty(a.label())) { + if (!a.label().empty()) { os << ", label="; detail::escape(os, a.label()); } @@ -38,7 +38,7 @@ inline std::ostream& operator<<(std::ostream& os, const circular_axis& os << ", phase=" << a.phase(); if (a.perimeter() != RealType(math::double_constants::two_pi)) os << ", perimeter=" << a.perimeter(); - if (!detail::empty(a.label())) { + if (!a.label().empty()) { os << ", label="; detail::escape(os, a.label()); } @@ -51,8 +51,8 @@ inline std::ostream& operator<<(std::ostream& os, const variable_axis& { os << "variable_axis(" << a[0]; for (int i = 1; i <= a.bins(); ++i) - os << ", " << a.left(i); - if (!detail::empty(a.label())) { + os << ", " << a[i]; + if (!a.label().empty()) { os << ", label="; detail::escape(os, a.label()); } @@ -65,7 +65,7 @@ inline std::ostream& operator<<(std::ostream& os, const variable_axis& inline std::ostream& operator<<(std::ostream& os, const integer_axis& a) { os << "integer_axis(" << a[0] << ", " << a[a.bins() - 1]; - if (!detail::empty(a.label())) { + if (!a.label().empty()) { os << ", label="; detail::escape(os, a.label()); } @@ -80,8 +80,13 @@ inline std::ostream& operator<<(std::ostream& os, const category_axis& a) os << "category_axis("; for (int i = 0; i < a.bins(); ++i) { detail::escape(os, a[i]); - os << (i == (a.bins() - 1)? ")" : ", "); + os << (i == (a.bins() - 1)? "" : ", "); } + if (!a.label().empty()) { + os << ", label="; + detail::escape(os, a.label()); + } + os << ")"; return os; } diff --git a/include/boost/histogram/detail/axis_visitor.hpp b/include/boost/histogram/detail/axis_visitor.hpp index 5322f231..bab06a84 100644 --- a/include/boost/histogram/detail/axis_visitor.hpp +++ b/include/boost/histogram/detail/axis_visitor.hpp @@ -45,7 +45,7 @@ namespace detail { const int i; left(const int x) : i(x) {} template - double operator()(const A& a) const { return a.left(i); } + double operator()(const A& a) const { return a[i]; } }; struct right : public static_visitor @@ -53,7 +53,7 @@ namespace detail { const int i; right(const int x) : i(x) {} template - double operator()(const A& a) const { return a.right(i); } + double operator()(const A& a) const { return a[i+1]; } }; struct center : public static_visitor @@ -61,7 +61,7 @@ namespace detail { const int i; center(const int x) : i(x) {} template - double operator()(const A& a) const { return 0.5 * (a.left(i) + a.right(i)); } + double operator()(const A& a) const { return 0.5 * (a[i] + a[i+1]); } }; struct cmp_axis : public static_visitor diff --git a/include/boost/histogram/detail/utility.hpp b/include/boost/histogram/detail/utility.hpp index 145d1fe7..2eeea2db 100644 --- a/include/boost/histogram/detail/utility.hpp +++ b/include/boost/histogram/detail/utility.hpp @@ -26,8 +26,6 @@ namespace detail { os << '\''; } - inline - bool empty(const std::string& s) { return s.empty(); } } } } diff --git a/include/boost/histogram/utility.hpp b/include/boost/histogram/utility.hpp index 12cb7255..a0cca3e6 100644 --- a/include/boost/histogram/utility.hpp +++ b/include/boost/histogram/utility.hpp @@ -35,21 +35,22 @@ int index(const boost::variant& a, const V v) { return apply_visitor(detail::index(v), a); } template -double left(const A& a, const int i) { return a.left(i); } +typename A::value_type left(const A& a, const int i) { return a[i]; } template double left(const boost::variant& a, const int i) { return apply_visitor(detail::left(i), a); } template -double right(const A& a, const int i) { return a.right(i); } +typename A::value_type right(const A& a, const int i) { return a[i+1]; } template double right(const boost::variant& a, const int i) { return apply_visitor(detail::right(i), a); } template -double center(const A& a, const int i) { return 0.5 * (a.left(i) + a.right(i)); } +typename A::value_type center(const A& a, const int i) +{ return 0.5 * (a[i] + a[i+1]); } template double center(const boost::variant& a, const int i) diff --git a/src/python/axis.cpp b/src/python/axis.cpp index 55c5e881..8cf3ed48 100644 --- a/src/python/axis.cpp +++ b/src/python/axis.cpp @@ -23,10 +23,8 @@ namespace { python::object variable_axis_init(python::tuple args, python::dict kwargs) { using namespace python; - using python::tuple; object self = args[0]; - object pyinit = self.attr("__init__"); if (len(args) < 2) { PyErr_SetString(PyExc_TypeError, "require at least two arguments"); @@ -41,7 +39,7 @@ variable_axis_init(python::tuple args, python::dict kwargs) { std::string label; bool uoflow = true; while (len(kwargs) > 0) { - tuple kv = kwargs.popitem(); + python::tuple kv = kwargs.popitem(); std::string k = extract(kv[0]); object v = kv[1]; if (k == "label") @@ -55,7 +53,8 @@ variable_axis_init(python::tuple args, python::dict kwargs) { throw_error_already_set(); } } - return pyinit(v, label, uoflow); + + return self.attr("__init__")(variable_axis<>(v.begin(), v.end(), label, uoflow)); } python::object @@ -63,33 +62,32 @@ category_axis_init(python::tuple args, python::dict kwargs) { using namespace python; object self = args[0]; - object pyinit = self.attr("__init__"); if (len(args) == 1) { PyErr_SetString(PyExc_TypeError, "require at least one argument"); throw_error_already_set(); } - if (len(kwargs) > 0) { - PyErr_SetString(PyExc_TypeError, "unknown keyword argument"); - throw_error_already_set(); + std::string label; + while (len(kwargs) > 0) { + python::tuple kv = kwargs.popitem(); + std::string k = extract(kv[0]); + object v = kv[1]; + if (k == "label") + label = extract(v); + else { + std::stringstream s; + s << "keyword " << k << " not recognized"; + PyErr_SetString(PyExc_KeyError, s.str().c_str()); + throw_error_already_set(); + } } - // if (len(args) == 2) { - // extract es(args[1]); - // if (es.check()) - // pyinit(es); - // else { - // PyErr_SetString(PyExc_TypeError, "require one or several string arguments"); - // throw_error_already_set(); - // } - // } - std::vector c; for (int i = 1, n = len(args); i < n; ++i) c.push_back(extract(args[i])); - return pyinit(c); + return self.attr("__init__")(category_axis(c.begin(), c.end(), label)); } template @@ -170,12 +168,6 @@ void register_axis_types() using python::arg; docstring_options dopt(true, true, false); - // used to pass arguments from raw python init to specialized C++ constructors - class_>("vector_double", no_init); - class_>("vector_string", no_init); - class_::const_iterator>("vector_double_iterator", no_init); - class_::const_iterator>("vector_string_iterator", no_init); - class_>("regular_axis", "An axis for real-valued data and bins of equal width." "\nBinning is a O(1) operation.", @@ -206,7 +198,7 @@ void register_axis_types() "\nthe problem domain allows it, prefer a regular_axis<>.", no_init) .def("__init__", raw_function(variable_axis_init)) - .def(init, const std::string&, bool>()) + .def(init&>()) .def(axis_suite>()) ; @@ -230,7 +222,7 @@ void register_axis_types() "\nBinning is a O(1) operation.", no_init) .def("__init__", raw_function(category_axis_init)) - .def(init>()) + .def(init()) .def(axis_suite()) ; } diff --git a/test/axis_test.cpp b/test/axis_test.cpp index 9bc95f6b..46fdf5c2 100644 --- a/test/axis_test.cpp +++ b/test/axis_test.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #define BOOST_TEST_NOT(expr) BOOST_TEST(!(expr)) @@ -17,8 +18,8 @@ template void test_real_axis_iterator(const Axis& a, int begin, int end) { for (const auto& bin : a) { BOOST_TEST_EQ(bin.idx, begin); - BOOST_TEST_EQ(bin.left, a.left(begin)); - BOOST_TEST_EQ(bin.right, a.right(begin)); + BOOST_TEST_EQ(bin.left, left(a, begin)); + BOOST_TEST_EQ(bin.right, right(a, begin)); ++begin; } BOOST_TEST_EQ(begin, end); diff --git a/test/python_suite_test.py.in b/test/python_suite_test.py.in index 45858d5d..956db150 100755 --- a/test/python_suite_test.py.in +++ b/test/python_suite_test.py.in @@ -219,6 +219,7 @@ class test_category_axis(unittest.TestCase): def test_init(self): category_axis("A", "B", "C") + category_axis("A", "B", "C", label="ca") with self.assertRaises(TypeError): category_axis() with self.assertRaises(TypeError): @@ -226,8 +227,8 @@ class test_category_axis(unittest.TestCase): with self.assertRaises(TypeError): category_axis("1", 2) with self.assertRaises(TypeError): - category_axis("A", "B", "C", label="ca") - with self.assertRaises(TypeError): + category_axis("A", "B", label=1) + with self.assertRaises(KeyError): category_axis("A", "B", "C", uoflow=True) self.assertEqual(category_axis("A", "B", "C"), category_axis("A", "B", "C"))