From 47250842641b4e027049d6c4957bd683ef9524d0 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 9 Apr 2024 14:07:21 +0200 Subject: [PATCH 01/11] Add get method returning an optional value This makes it possible to distinguish between set and empty values and non-existant values. Deprecate getValue and setValue and also introduce set for symmetry. --- include/podio/GenericParameters.h | 71 ++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 3e1700549..66fa78759 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -41,8 +42,8 @@ using EnableIfValidGenericDataType = typename std::enable_if_t struct GenericDataReturnTypeHelper { using type = T; @@ -63,8 +64,10 @@ namespace detail { /// Alias template for determining the appropriate return type for the passed in /// type + template -using GenericDataReturnType = typename detail::GenericDataReturnTypeHelper::type; +using GenericDataReturnType [[deprecated("GenericParameters will return by optional soon")]] = + typename detail::GenericDataReturnTypeHelper::type; /// GenericParameters objects allow one to store generic named parameters of type /// int, float and string or vectors of these types. @@ -98,29 +101,54 @@ class GenericParameters { ~GenericParameters() = default; + template > + std::optional get(const std::string& key) const; + /// Get the value that is stored under the given key, by const reference or by /// value depending on the desired type template > - GenericDataReturnType getValue(const std::string&) const; + [[deprecated("Use 'get' instead")]] GenericDataReturnType getValue(const std::string&) const; /// Store (a copy of) the passed value under the given key template > - void setValue(const std::string& key, T value); + [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, T value) { + set(key, value); + } /// Overload for catching const char* setting for string values - void setValue(const std::string& key, std::string value) { - setValue(key, std::move(value)); + [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, std::string value) { + set(key, std::move(value)); } /// Overload for catching initializer list setting of string vector values - void setValue(const std::string& key, std::vector values) { - setValue>(key, std::move(values)); + [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, std::vector values) { + set>(key, std::move(values)); } /// Overload for catching initializer list setting for vector values template >> void setValue(const std::string& key, std::initializer_list&& values) { - setValue>(key, std::move(values)); + set>(key, std::move(values)); + } + + /// Store (a copy of) the passed value under the given key + template > + void set(const std::string& key, T value); + + /// Overload for catching const char* setting for string values + void set(const std::string& key, std::string value) { + set(key, std::move(value)); + } + + /// Overload for catching initializer list setting of string vector values + void set(const std::string& key, std::vector values) { + set>(key, std::move(values)); + } + + /// Overload for catching initializer list setting for vector values + template >> + void set(const std::string& key, std::initializer_list&& values) { + set>(key, std::move(values)); } /// Get the number of elements stored under the given key for a type @@ -145,8 +173,10 @@ class GenericParameters { return _intMap.empty() && _floatMap.empty() && _stringMap.empty(); } +#if PODIO_ENABLE_SIO friend void writeGenericParameters(sio::write_device& device, const GenericParameters& parameters); friend void readGenericParameters(sio::read_device& device, GenericParameters& parameters, sio::version_type version); +#endif #if PODIO_ENABLE_RNTUPLE friend RNTupleReader; @@ -208,6 +238,25 @@ class GenericParameters { mutable MutexPtr m_doubleMtx{nullptr}; ///< The mutex guarding the double map }; +template +std::optional GenericParameters::get(const std::string& key) const { + const auto& map = getMap(); + auto& mtx = getMutex(); + std::lock_guard lock{mtx}; + const auto it = map.find(key); + if (it == map.end()) { + return std::nullopt; + } + + // We have to check whether the return type is a vector or a single value + if constexpr (detail::isVector) { + return it->second; + } else { + const auto& iv = it->second; + return iv[0]; + } +} + template GenericDataReturnType GenericParameters::getValue(const std::string& key) const { const auto& map = getMap(); @@ -231,7 +280,7 @@ GenericDataReturnType GenericParameters::getValue(const std::string& key) con } template -void GenericParameters::setValue(const std::string& key, T value) { +void GenericParameters::set(const std::string& key, T value) { auto& map = getMap(); auto& mtx = getMutex(); From cd5017df3ee6c63e42d03ad5a4613eb5837ff1b9 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 9 Apr 2024 14:18:52 +0200 Subject: [PATCH 02/11] Fix unittests to use non-deprecated methods --- tests/unittests/unittest.cpp | 95 ++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 48 deletions(-) diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 934577bb3..e44577fa8 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -1007,35 +1007,35 @@ TEST_CASE("GenericParameters", "[generic-parameters]") { // Check that GenericParameters work as intended auto gp = podio::GenericParameters{}; - gp.setValue("anInt", 42); - REQUIRE(gp.getValue("anInt") == 42); + gp.set("anInt", 42); + REQUIRE(*gp.get("anInt") == 42); // Make sure that resetting a value with the same key works - gp.setValue("anInt", -42); - REQUIRE(gp.getValue("anInt") == -42); + gp.set("anInt", -42); + REQUIRE(*gp.get("anInt") == -42); // Make sure that passing a string literal is converted to a string on the fly - gp.setValue("aString", "const char initialized"); - REQUIRE(gp.getValue("aString") == "const char initialized"); + gp.set("aString", "const char initialized"); + REQUIRE(*gp.get("aString") == "const char initialized"); - gp.setValue("aStringVec", {"init", "from", "const", "chars"}); - const auto& stringVec = gp.getValue>("aStringVec"); + gp.set("aStringVec", {"init", "from", "const", "chars"}); + const auto stringVec = gp.get>("aStringVec").value(); REQUIRE(stringVec.size() == 4); REQUIRE(stringVec[0] == "init"); REQUIRE(stringVec[3] == "chars"); // Check that storing double values works - gp.setValue("double", 1.234); - gp.setValue("manyDoubles", {1.23, 4.56, 7.89}); - REQUIRE(gp.getValue("double") == 1.234); - const auto& storedDoubles = gp.getValue>("manyDoubles"); + gp.set("double", 1.234); + gp.set("manyDoubles", {1.23, 4.56, 7.89}); + REQUIRE(gp.get("double") == 1.234); + const auto storedDoubles = gp.get>("manyDoubles").value(); REQUIRE(storedDoubles.size() == 3); REQUIRE(storedDoubles[0] == 1.23); REQUIRE(storedDoubles[1] == 4.56); REQUIRE(storedDoubles[2] == 7.89); // Check that passing an initializer_list creates the vector on the fly - gp.setValue("manyInts", {1, 2, 3, 4}); - const auto& ints = gp.getValue>("manyInts"); + gp.set("manyInts", {1, 2, 3, 4}); + const auto ints = gp.get>("manyInts").value(); REQUIRE(ints.size() == 4); for (int i = 0; i < 4; ++i) { REQUIRE(ints[i] == i + 1); @@ -1043,72 +1043,71 @@ TEST_CASE("GenericParameters", "[generic-parameters]") { auto floats = std::vector{3.14f, 2.718f}; // This stores a copy of the current value - gp.setValue("someFloats", floats); + gp.set("someFloats", floats); // Hence, modifying the original vector will not be reflected floats.push_back(42.f); REQUIRE(floats.size() == 3); - const auto& storedFloats = gp.getValue>("someFloats"); + const auto storedFloats = gp.get>("someFloats").value(); REQUIRE(storedFloats.size() == 2); REQUIRE(storedFloats[0] == 3.14f); REQUIRE(storedFloats[1] == 2.718f); // We can at this point reset this to a single value with the same key even if // it has been a vector before - gp.setValue("someFloats", 12.34f); - REQUIRE(gp.getValue("someFloats") == 12.34f); + gp.set("someFloats", 12.34f); + REQUIRE(*gp.get("someFloats") == 12.34f); - // Missing values return the default initialized ones - REQUIRE(gp.getValue("MissingValue") == int{}); - REQUIRE(gp.getValue("MissingValue") == float{}); - REQUIRE(gp.getValue("MissingValue") == std::string{}); // NOLINT(readability-container-size-empty): We - // want the explicit comparison here + // Missing values return an empty optional + REQUIRE_FALSE(gp.get("Missing")); + REQUIRE_FALSE(gp.get("Missing")); + REQUIRE_FALSE(gp.get("Missing")); // Same for vectors - REQUIRE(gp.getValue>("MissingValue").empty()); - REQUIRE(gp.getValue>("MissingValue").empty()); - REQUIRE(gp.getValue>("MissingValue").empty()); + REQUIRE_FALSE(gp.get>("Missing")); + REQUIRE_FALSE(gp.get>("Missing")); + REQUIRE_FALSE(gp.get>("Missing")); } TEST_CASE("GenericParameters constructors", "[generic-parameters]") { // Tests for making sure that generic parameters can be moved / copied correctly auto originalParams = podio::GenericParameters{}; - originalParams.setValue("int", 42); - originalParams.setValue("ints", {1, 2}); - originalParams.setValue("float", 3.14f); - originalParams.setValue("double", 2 * 3.14); - originalParams.setValue("strings", {"one", "two", "three"}); + originalParams.set("int", 42); + originalParams.set("ints", {1, 2}); + originalParams.set("float", 3.14f); + originalParams.set("double", 2 * 3.14); + originalParams.set("strings", {"one", "two", "three"}); SECTION("Copy constructor") { auto copiedParams = originalParams; - REQUIRE(copiedParams.getValue("int") == 42); - REQUIRE(copiedParams.getValue>("ints")[1] == 2); - REQUIRE(copiedParams.getValue("float") == 3.14f); - REQUIRE(copiedParams.getValue("double") == 2 * 3.14); - REQUIRE(copiedParams.getValue>("strings")[0] == "one"); + REQUIRE(*copiedParams.get("int") == 42); + REQUIRE(copiedParams.get>("ints").value()[1] == 2); + REQUIRE(*copiedParams.get("float") == 3.14f); + REQUIRE(*copiedParams.get("double") == 2 * 3.14); + REQUIRE(copiedParams.get>("strings").value()[0] == "one"); // Make sure these are truly independent copies now - copiedParams.setValue("anotherDouble", 1.2345); - REQUIRE(originalParams.getValue("anotherDouble") == double{}); + copiedParams.set("anotherDouble", 1.2345); + REQUIRE_FALSE(originalParams.get("anotherDouble")); } SECTION("Move constructor") { auto copiedParams = std::move(originalParams); - REQUIRE(copiedParams.getValue("int") == 42); - REQUIRE(copiedParams.getValue>("ints")[1] == 2); - REQUIRE(copiedParams.getValue("float") == 3.14f); - REQUIRE(copiedParams.getValue("double") == 2 * 3.14); - REQUIRE(copiedParams.getValue>("strings")[0] == "one"); + REQUIRE(copiedParams.get("int") == 42); + REQUIRE(copiedParams.get>("ints").value()[1] == 2); + REQUIRE(copiedParams.get("float") == 3.14f); + REQUIRE(copiedParams.get("double") == 2 * 3.14); + REQUIRE(copiedParams.get>("strings").value()[0] == "one"); } SECTION("Move assignment") { auto copiedParams = podio::GenericParameters{}; copiedParams = std::move(originalParams); - REQUIRE(copiedParams.getValue("int") == 42); - REQUIRE(copiedParams.getValue>("ints")[1] == 2); - REQUIRE(copiedParams.getValue("float") == 3.14f); - REQUIRE(copiedParams.getValue("double") == 2 * 3.14); - REQUIRE(copiedParams.getValue>("strings")[0] == "one"); + REQUIRE(copiedParams.get("int") == 42); + REQUIRE(copiedParams.get>("ints").value()[1] == 2); + REQUIRE(copiedParams.get("float") == 3.14f); + REQUIRE(copiedParams.get("double") == 2 * 3.14); + REQUIRE(copiedParams.get>("strings").value()[0] == "one"); } } From e91faa0c25e026c87f72c91b0d4e343c2ce41d2b Mon Sep 17 00:00:00 2001 From: tmadlener Date: Tue, 9 Apr 2024 14:19:21 +0200 Subject: [PATCH 03/11] Remove test of deprecated type helper --- tests/unittests/unittest.cpp | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index e44577fa8..8182772de 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -1111,25 +1111,6 @@ TEST_CASE("GenericParameters constructors", "[generic-parameters]") { } } -// Helper alias template "macro" to get the return type of calling -// GenericParameters::getValue with the desired template type -template -using GPGetValue = decltype(std::declval().getValue(std::declval())); - -TEST_CASE("GenericParameters return types", "[generic-parameters][static-checks]") { - // Tests for checking that the getValue returns return by value resp. by const - // reference as expected - STATIC_REQUIRE(std::is_same_v, int>); // int and float are returned by value - STATIC_REQUIRE(std::is_same_v>, const std::vector&>); // vectors are const - // references - - STATIC_REQUIRE(std::is_same_v, const std::string&>); // std::strings are returned by const - // reference as well - STATIC_REQUIRE(std::is_same_v>, const std::vector&>); // as are - // vectors of - // strings -} - TEST_CASE("Missing files (ROOT readers)", "[basics]") { auto root_legacy_reader = podio::ROOTLegacyReader(); REQUIRE_THROWS_AS(root_legacy_reader.openFile("NonExistentFile.root"), std::runtime_error); From c19d01cd4c729f5838cff6396b867f8f90715eff Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 10 Apr 2024 13:06:22 +0200 Subject: [PATCH 04/11] Switch to non-deprecated usage in Frame --- include/podio/Frame.h | 6 +++--- python/podio/frame.py | 4 +++- tests/unittests/frame.cpp | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/podio/Frame.h b/include/podio/Frame.h index ce459e119..0d8a4cbb5 100644 --- a/include/podio/Frame.h +++ b/include/podio/Frame.h @@ -236,7 +236,7 @@ class Frame { /// @param value The value of the parameter. A copy will be put into the Frame template > inline void putParameter(const std::string& key, T value) { - m_self->parameters().setValue(key, std::move(value)); + m_self->parameters().set(key, std::move(value)); } /// Add a string value to the parameters of the Frame. @@ -286,8 +286,8 @@ class Frame { /// /// @returns The value of the parameter or an empty default value template > - inline podio::GenericDataReturnType getParameter(const std::string& key) const { - return m_self->parameters().getValue(key); + inline auto getParameter(const std::string& key) const { + return m_self->parameters().get(key).value_or(T{}); } /// Retrieve all parameters stored in this Frame. diff --git a/python/podio/frame.py b/python/podio/frame.py index faf886e35..c467d281e 100644 --- a/python/podio/frame.py +++ b/python/podio/frame.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 """Module for the python bindings of the podio::Frame""" +from copy import deepcopy + import cppyy import ROOT @@ -193,7 +195,7 @@ def _get_param_value(par_type, name): par_value = self._frame.getParameter[par_type](name) if len(par_value) == 1: return par_value[0] - return list(par_value) + return list(deepcopy(par_value)) # This access already raises the KeyError if there is no such parameter par_type = self._param_key_types[name] diff --git a/tests/unittests/frame.cpp b/tests/unittests/frame.cpp index a5159de72..7714851f2 100644 --- a/tests/unittests/frame.cpp +++ b/tests/unittests/frame.cpp @@ -239,7 +239,7 @@ void checkFrame(const podio::Frame& frame) { REQUIRE(clusters[1].Clusters()[0] == clusters[0]); REQUIRE(frame.getParameter("anInt") == 42); - auto& floats = frame.getParameter>("someFloats"); + auto floats = frame.getParameter>("someFloats"); REQUIRE(floats.size() == 3); REQUIRE(floats[0] == 1.23f); REQUIRE(floats[1] == 2.34f); From 541bd00ee44feb512b280a5a286dce4d7dda4b96 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 10 Apr 2024 13:58:42 +0200 Subject: [PATCH 05/11] Make the Frame return optional parameters as well --- include/podio/Frame.h | 4 ++-- python/podio/frame.py | 4 +++- tests/read_python_frame.h | 10 +++++----- tests/read_test.h | 8 ++++---- tests/unittests/frame.cpp | 8 ++++---- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/podio/Frame.h b/include/podio/Frame.h index 0d8a4cbb5..7c328afc6 100644 --- a/include/podio/Frame.h +++ b/include/podio/Frame.h @@ -284,10 +284,10 @@ class Frame { /// @tparam T The desired type of the parameter (can also be std::vector) /// @param key The key under which the value is stored /// - /// @returns The value of the parameter or an empty default value + /// @returns An optional holding the value if it was present template > inline auto getParameter(const std::string& key) const { - return m_self->parameters().get(key).value_or(T{}); + return m_self->parameters().get(key); } /// Retrieve all parameters stored in this Frame. diff --git a/python/podio/frame.py b/python/podio/frame.py index c467d281e..6a8ed3bc4 100644 --- a/python/podio/frame.py +++ b/python/podio/frame.py @@ -192,7 +192,9 @@ def get_parameter(self, name, as_type=None): """ def _get_param_value(par_type, name): - par_value = self._frame.getParameter[par_type](name) + # We can safely assume that getting the value here works, because + # only valid keys will end up here + par_value = self._frame.getParameter[par_type](name).value() if len(par_value) == 1: return par_value[0] return list(deepcopy(par_value)) diff --git a/tests/read_python_frame.h b/tests/read_python_frame.h index 318482ed9..5c3220708 100644 --- a/tests/read_python_frame.h +++ b/tests/read_python_frame.h @@ -56,34 +56,34 @@ std::ostream& operator<<(std::ostream& o, const std::vector& vec) { } int checkParameters(const podio::Frame& frame) { - const auto iVal = frame.getParameter("an_int"); + const auto iVal = frame.getParameter("an_int").value(); if (iVal != 42) { std::cerr << "Parameter an_int was not stored correctly (expected 42, actual " << iVal << ")" << std::endl; return 1; } - const auto& dVal = frame.getParameter>("some_floats"); + const auto dVal = frame.getParameter>("some_floats").value(); if (dVal.size() != 3 || dVal[0] != 1.23 || dVal[1] != 7.89 || dVal[2] != 3.14) { std::cerr << "Parameter some_floats was not stored correctly (expected [1.23, 7.89, 3.14], actual " << dVal << ")" << std::endl; return 1; } - const auto& strVal = frame.getParameter>("greetings"); + const auto strVal = frame.getParameter>("greetings").value(); if (strVal.size() != 2 || strVal[0] != "from" || strVal[1] != "python") { std::cerr << "Parameter greetings was not stored correctly (expected [from, python], actual " << strVal << ")" << std::endl; return 1; } - const auto realFloat = frame.getParameter("real_float"); + const auto realFloat = frame.getParameter("real_float").value_or(-1.f); if (realFloat != 3.14f) { std::cerr << "Parameter real_float was not stored correctly (expected 3.14, actual " << realFloat << ")" << std::endl; return 1; } - const auto& realFloats = frame.getParameter>("more_real_floats"); + const auto realFloats = frame.getParameter>("more_real_floats").value(); if (realFloats.size() != 3 || realFloats[0] != 1.23f || realFloats[1] != 4.56f || realFloats[2] != 7.89f) { std::cerr << "Parameter more_real_floats was not stored as correctly (expected [1.23, 4.56, 7.89], actual" << realFloats << ")" << std::endl; diff --git a/tests/read_test.h b/tests/read_test.h index 2d515e1c2..5c441dfcd 100644 --- a/tests/read_test.h +++ b/tests/read_test.h @@ -39,7 +39,7 @@ bool check_fixed_width_value(FixedWidthT actual, FixedWidthT expected, const std } void processEvent(const podio::Frame& event, int eventNum, podio::version::Version fileVersion) { - const float evtWeight = event.getParameter("UserEventWeight"); + const float evtWeight = event.getParameter("UserEventWeight").value(); if (evtWeight != (float)100. * eventNum) { std::cout << " read UserEventWeight: " << evtWeight << " - expected : " << (float)100. * eventNum << std::endl; throw std::runtime_error("Couldn't read event meta data parameters 'UserEventWeight'"); @@ -47,7 +47,7 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi std::stringstream ss; ss << " event_number_" << eventNum; - const auto& evtName = event.getParameter("UserEventName"); + const auto evtName = event.getParameter("UserEventName").value(); if (evtName != ss.str()) { std::cout << " read UserEventName: " << evtName << " - expected : " << ss.str() << std::endl; @@ -55,7 +55,7 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi } if (fileVersion > podio::version::Version{0, 14, 1}) { - const auto& someVectorData = event.getParameter>("SomeVectorData"); + const auto someVectorData = event.getParameter>("SomeVectorData").value(); if (someVectorData.size() != 4) { throw std::runtime_error("Couldn't read event meta data parameters: 'SomeVectorData'"); } @@ -67,7 +67,7 @@ void processEvent(const podio::Frame& event, int eventNum, podio::version::Versi } if (fileVersion > podio::version::Version{0, 16, 2}) { - const auto& doubleParams = event.getParameter>("SomeVectorData"); + const auto doubleParams = event.getParameter>("SomeVectorData").value(); if (doubleParams.size() != 2 || doubleParams[0] != eventNum * 1.1 || doubleParams[1] != eventNum * 2.2) { throw std::runtime_error("Could not read event parameter: 'SomeDoubleValues' correctly"); } diff --git a/tests/unittests/frame.cpp b/tests/unittests/frame.cpp index 7714851f2..ac014a898 100644 --- a/tests/unittests/frame.cpp +++ b/tests/unittests/frame.cpp @@ -33,13 +33,13 @@ TEST_CASE("Frame parameters", "[frame][basics]") { REQUIRE(event.getParameter("aString") == "from a string literal"); event.putParameter("someInts", {42, 123}); - const auto& ints = event.getParameter>("someInts"); + const auto ints = event.getParameter>("someInts").value(); REQUIRE(ints.size() == 2); REQUIRE(ints[0] == 42); REQUIRE(ints[1] == 123); event.putParameter("someStrings", {"one", "two", "three"}); - const auto& strings = event.getParameter>("someStrings"); + const auto strings = event.getParameter>("someStrings").value(); REQUIRE(strings.size() == 3); REQUIRE(strings[0] == "one"); REQUIRE(strings[1] == "two"); @@ -239,7 +239,7 @@ void checkFrame(const podio::Frame& frame) { REQUIRE(clusters[1].Clusters()[0] == clusters[0]); REQUIRE(frame.getParameter("anInt") == 42); - auto floats = frame.getParameter>("someFloats"); + const auto floats = frame.getParameter>("someFloats").value(); REQUIRE(floats.size() == 3); REQUIRE(floats[0] == 1.23f); REQUIRE(floats[1] == 2.34f); @@ -339,7 +339,7 @@ TEST_CASE("Frame parameters multithread insert and read", "[frame][basics][multi frame.putParameter(makeName("string", i), std::to_string(i)); CHECK_INCREASE(frame.getParameter("string_par") == "some string", successes[i]); - const auto& floatPars = frame.getParameter>("float_pars"); + const auto floatPars = frame.getParameter>("float_pars").value(); CHECK_INCREASE(floatPars.size() == 3, successes[i]); CHECK_INCREASE(floatPars[0] == 1.23f, successes[i]); CHECK_INCREASE(floatPars[1] == 4.56f, successes[i]); From 19a3c5a9f3ceba7ff774f7aa1cacae9e05eb6624 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 10 Apr 2024 14:03:21 +0200 Subject: [PATCH 06/11] Remove deprecated functionality from GenericParameters --- include/podio/GenericParameters.h | 49 ------------------------------- 1 file changed, 49 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 66fa78759..c93f30f3c 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -104,33 +104,6 @@ class GenericParameters { template > std::optional get(const std::string& key) const; - /// Get the value that is stored under the given key, by const reference or by - /// value depending on the desired type - template > - [[deprecated("Use 'get' instead")]] GenericDataReturnType getValue(const std::string&) const; - - /// Store (a copy of) the passed value under the given key - template > - [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, T value) { - set(key, value); - } - - /// Overload for catching const char* setting for string values - [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, std::string value) { - set(key, std::move(value)); - } - - /// Overload for catching initializer list setting of string vector values - [[deprecated("Use 'set' instead")]] void setValue(const std::string& key, std::vector values) { - set>(key, std::move(values)); - } - - /// Overload for catching initializer list setting for vector values - template >> - void setValue(const std::string& key, std::initializer_list&& values) { - set>(key, std::move(values)); - } - /// Store (a copy of) the passed value under the given key template > void set(const std::string& key, T value); @@ -257,28 +230,6 @@ std::optional GenericParameters::get(const std::string& key) const { } } -template -GenericDataReturnType GenericParameters::getValue(const std::string& key) const { - const auto& map = getMap(); - auto& mtx = getMutex(); - std::lock_guard lock{mtx}; - const auto it = map.find(key); - // If there is no entry to the key, we just return an empty default - // TODO: make this case detectable from the outside - if (it == map.end()) { - static const auto empty = T{}; - return empty; - } - - // We have to check whether the return type is a vector or a single value - if constexpr (detail::isVector) { - return it->second; - } else { - const auto& iv = it->second; - return iv[0]; - } -} - template void GenericParameters::set(const std::string& key, T value) { auto& map = getMap(); From d08f43659e3fd25fd2e43c75cd2ce85172760030 Mon Sep 17 00:00:00 2001 From: tmadlener Date: Wed, 10 Apr 2024 14:03:21 +0200 Subject: [PATCH 07/11] Remove deprecated functionality from GenericParameters --- include/podio/GenericParameters.h | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index c93f30f3c..6eb6634c9 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -40,35 +40,6 @@ static constexpr bool isSupportedGenericDataType = detail::isAnyOrVectorOf using EnableIfValidGenericDataType = typename std::enable_if_t>; -namespace detail { - /// Helper struct to determine how to return different types from the - /// GenericParameters to avoid unnecessary copies but also to make it possible - /// do distinguish between set and unset - template - struct GenericDataReturnTypeHelper { - using type = T; - }; - - /// Specialization for std::string. Those will always be returned by const ref - template <> - struct GenericDataReturnTypeHelper { - using type = const std::string&; - }; - - /// Specialization for std::vector. Those will always be returned by const ref - template - struct GenericDataReturnTypeHelper> { - using type = const std::vector&; - }; -} // namespace detail - -/// Alias template for determining the appropriate return type for the passed in -/// type - -template -using GenericDataReturnType [[deprecated("GenericParameters will return by optional soon")]] = - typename detail::GenericDataReturnTypeHelper::type; - /// GenericParameters objects allow one to store generic named parameters of type /// int, float and string or vectors of these types. /// They can be used to store (user) meta data that is From ad756577c40e73da58b1f58e6fa5fbfcda363c2b Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 16 May 2024 10:23:30 +0200 Subject: [PATCH 08/11] Make getMap private as public access is no longer necessary --- include/podio/GenericParameters.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 6eb6634c9..9f403666b 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -13,11 +13,13 @@ #include #include +#if PODIO_ENABLE_SIO namespace sio { class read_device; class write_device; using version_type = uint32_t; // from sio/definitions } // namespace sio +#endif #if PODIO_ENABLE_RNTUPLE namespace podio { @@ -127,9 +129,10 @@ class GenericParameters { friend RNTupleWriter; #endif +private: /// Get a reference to the internal map for a given type template - const MapType>& getMap() const { + MapType>& getMap() { if constexpr (std::is_same_v, int>) { return _intMap; } else if constexpr (std::is_same_v, float>) { @@ -141,10 +144,9 @@ class GenericParameters { } } -private: - /// Get a reference to the internal map for a given type (necessary for SIO) + /// Get a reference to the internal map for a given type template - MapType>& getMap() { + const MapType>& getMap() const { if constexpr (std::is_same_v, int>) { return _intMap; } else if constexpr (std::is_same_v, float>) { @@ -156,7 +158,6 @@ class GenericParameters { } } -private: /// Get the mutex that guards the map for the given type template std::mutex& getMutex() const { From 170e24213c98f8cdd41ff97bdd6ab407706e70fa Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 16 May 2024 10:35:32 +0200 Subject: [PATCH 09/11] Bump the patch version to 99 to signify development --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 43a1cd67f..56fec26ef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ project(podio) #--- Version ------------------------------------------------------------------- SET( ${PROJECT_NAME}_VERSION_MAJOR 0 ) SET( ${PROJECT_NAME}_VERSION_MINOR 99 ) -SET( ${PROJECT_NAME}_VERSION_PATCH 0 ) +SET( ${PROJECT_NAME}_VERSION_PATCH 99 ) SET( ${PROJECT_NAME}_VERSION "${${PROJECT_NAME}_VERSION_MAJOR}.${${PROJECT_NAME}_VERSION_MINOR}.${${PROJECT_NAME}_VERSION_PATCH}" ) From eeadbf779cf7ee3d5362c5892acfb1ce2005107f Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 16 May 2024 11:04:51 +0200 Subject: [PATCH 10/11] Make map public again, as it is needed for DD4hep --- include/podio/GenericParameters.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/podio/GenericParameters.h b/include/podio/GenericParameters.h index 9f403666b..d3f2f394f 100644 --- a/include/podio/GenericParameters.h +++ b/include/podio/GenericParameters.h @@ -129,10 +129,9 @@ class GenericParameters { friend RNTupleWriter; #endif -private: /// Get a reference to the internal map for a given type template - MapType>& getMap() { + const MapType>& getMap() const { if constexpr (std::is_same_v, int>) { return _intMap; } else if constexpr (std::is_same_v, float>) { @@ -144,9 +143,10 @@ class GenericParameters { } } +private: /// Get a reference to the internal map for a given type template - const MapType>& getMap() const { + MapType>& getMap() { if constexpr (std::is_same_v, int>) { return _intMap; } else if constexpr (std::is_same_v, float>) { From c0d59079cf168127f2b0cc12b3d30d23b0cb587a Mon Sep 17 00:00:00 2001 From: tmadlener Date: Thu, 16 May 2024 11:11:30 +0200 Subject: [PATCH 11/11] Fix comments on documentation --- include/podio/Frame.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/include/podio/Frame.h b/include/podio/Frame.h index 7c328afc6..75aedc3bd 100644 --- a/include/podio/Frame.h +++ b/include/podio/Frame.h @@ -278,13 +278,10 @@ class Frame { /// Retrieve parameters via key from the internal store. /// - /// The return type will either be a const reference or a value depending on - /// the desired type. See podio::GenericParameters for more details. - /// /// @tparam T The desired type of the parameter (can also be std::vector) /// @param key The key under which the value is stored /// - /// @returns An optional holding the value if it was present + /// @returns An optional holding the value if it is present template > inline auto getParameter(const std::string& key) const { return m_self->parameters().get(key); @@ -293,7 +290,7 @@ class Frame { /// Retrieve all parameters stored in this Frame. /// /// This is mainly intended for I/O purposes and we encourage to use the Frame - /// functionality of getParameters or getParameterKeys in general. + /// functionality of getParameter or getParameterKeys in general. /// /// @returns The internally used GenericParameters inline const podio::GenericParameters& getParameters() const {