From 869e3229456d2723974ae4b755fa0a6776f40be4 Mon Sep 17 00:00:00 2001 From: Sylwester Arabas Date: Sat, 2 Dec 2023 22:09:02 +0100 Subject: [PATCH 1/3] raise exception instead of segfault for non-unique mass_frac species in AeroMode and AeroDist ctors. closes #240 --- src/aero_dist.hpp | 2 ++ src/aero_mode.hpp | 18 +++++++++++++++--- tests/test_aero_dist.py | 16 ++++++++++++++++ tests/test_aero_mode.py | 26 +++++++++++++++++++------- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/aero_dist.hpp b/src/aero_dist.hpp index cab50f17..a2ded63c 100644 --- a/src/aero_dist.hpp +++ b/src/aero_dist.hpp @@ -51,6 +51,8 @@ struct AeroDist { { if (!InputGimmick::unique_keys(json)) throw std::runtime_error("Mode names must be unique"); + for (const auto mode : json) + AeroMode::check_mode_json(mode.begin().value()); GimmickGuard guard(json, "", "mode_name", 1); f_aero_dist_from_json(ptr.f_arg_non_const(), aero_data->ptr.f_arg_non_const()); diff --git a/src/aero_mode.hpp b/src/aero_mode.hpp index e8ac50c8..145bb044 100644 --- a/src/aero_mode.hpp +++ b/src/aero_mode.hpp @@ -128,13 +128,25 @@ struct AeroMode { AeroMode(AeroData &aero_data, const nlohmann::json &json) : ptr(f_aero_mode_ctor, f_aero_mode_dtor) { - if (json.size() != 1) - throw std::runtime_error("Single element expected"); - + if (json.size() != 1 || !json.is_object() || !json.begin().value().is_object()) + throw std::runtime_error("Single-element dict expected with mode name as key and mode params dict as value"); + check_mode_json(json.begin().value()); GimmickGuard guard(json, "", "mode_name"); f_aero_mode_from_json(ptr.f_arg_non_const(), aero_data.ptr.f_arg_non_const()); } + static void check_mode_json(const nlohmann::json &mode) { + for (auto key : std::set({"mass_frac", "mode_type"})) // TODO #320: more... + if (mode.find(key) == mode.end()) + throw std::runtime_error("mode parameters dict must include key '" + key + "'"); + auto mass_frac = mode["mass_frac"]; + + if (!mass_frac.is_array()) // TODO #320: check if all are single-element dicts + throw std::runtime_error("mass_frac value must be a list of single-element dicts"); + if (!InputGimmick::unique_keys(mass_frac)) + throw std::runtime_error("mass_frac keys must be unique"); + } + static auto get_num_conc(const AeroMode &self){ double val; f_aero_mode_get_num_conc(self.ptr.f_arg(), &val); diff --git a/tests/test_aero_dist.py b/tests/test_aero_dist.py index b02dc0f9..d8f03a01 100644 --- a/tests/test_aero_dist.py +++ b/tests/test_aero_dist.py @@ -163,3 +163,19 @@ def test_ctor_multimode_error_on_repeated_mode_names(): # assert assert str(exc_info.value) == "Mode names must be unique" + + @staticmethod + def test_ctor_error_on_repeated_massfrac_keys(): + # arrange + aero_data = ppmc.AeroData(AERO_DATA_CTOR_ARG_MINIMAL) + fishy_ctor_arg = copy.deepcopy(AERO_DIST_CTOR_ARG_MINIMAL) + fishy_ctor_arg[0]["test_mode"]["mass_frac"].append( + fishy_ctor_arg[0]["test_mode"]["mass_frac"][0] + ) + + # act + with pytest.raises(Exception) as exc_info: + ppmc.AeroDist(aero_data, fishy_ctor_arg) + + # assert + assert str(exc_info.value) == "mass_frac keys must be unique" diff --git a/tests/test_aero_mode.py b/tests/test_aero_mode.py index e927e674..ab636fb3 100644 --- a/tests/test_aero_mode.py +++ b/tests/test_aero_mode.py @@ -4,6 +4,7 @@ # Authors: https://github.com/open-atmos/PyPartMC/graphs/contributors # #################################################################################################### +import copy import numpy as np import pytest @@ -242,7 +243,7 @@ def test_set_name(): def test_ctor_fails_with_multiple_modes(): # arrange aero_data = ppmc.AeroData(AERO_DATA_CTOR_ARG_MINIMAL) - fishy_ctor_arg = AERO_MODE_CTOR_LOG_NORMAL + fishy_ctor_arg = copy.deepcopy(AERO_MODE_CTOR_LOG_NORMAL) fishy_ctor_arg["xxx"] = fishy_ctor_arg["test_mode"] # act @@ -250,17 +251,15 @@ def test_ctor_fails_with_multiple_modes(): ppmc.AeroMode(aero_data, fishy_ctor_arg) # assert - assert str(exc_info.value) == "Single element expected" + assert str(exc_info.value) == "Single-element dict expected with mode name as key and mode params dict as value" @staticmethod def test_ctor_fails_with_nonunique_mass_fracs(): - pytest.skip("TODO #240") - # arrange aero_data = ppmc.AeroData(AERO_DATA_CTOR_ARG_MINIMAL) - fishy_ctor_arg = AERO_MODE_CTOR_LOG_NORMAL + fishy_ctor_arg = copy.deepcopy(AERO_MODE_CTOR_LOG_NORMAL) fishy_ctor_arg["test_mode"]["mass_frac"].append( - fishy_ctor_arg["test_mode"]["mass_frac"] + AERO_MODE_CTOR_LOG_NORMAL["test_mode"]["mass_frac"][0] ) # act @@ -268,4 +267,17 @@ def test_ctor_fails_with_nonunique_mass_fracs(): ppmc.AeroMode(aero_data, fishy_ctor_arg) # assert - assert str(exc_info.value) == "" + assert str(exc_info.value) == "mass_frac keys must be unique" + + @staticmethod + def test_segfault_case(): # TODO #319 + pytest.skip() + + aero_data = ppmc.AeroData(AERO_DATA_CTOR_ARG_MINIMAL) + fishy_ctor_arg = copy.deepcopy(AERO_MODE_CTOR_LOG_NORMAL) + fishy_ctor_arg["test_mode"]["mass_frac"].append( + fishy_ctor_arg["test_mode"]["mass_frac"] + ) + print(fishy_ctor_arg) + ppmc.AeroMode(aero_data, fishy_ctor_arg) + From 20d1b5307442fefcd922287dca2d119c8804139e Mon Sep 17 00:00:00 2001 From: Sylwester Arabas Date: Sat, 2 Dec 2023 22:13:34 +0100 Subject: [PATCH 2/3] forgot pre-commit --- tests/test_aero_mode.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_aero_mode.py b/tests/test_aero_mode.py index ab636fb3..523e20fa 100644 --- a/tests/test_aero_mode.py +++ b/tests/test_aero_mode.py @@ -5,6 +5,7 @@ #################################################################################################### import copy + import numpy as np import pytest @@ -251,7 +252,10 @@ def test_ctor_fails_with_multiple_modes(): ppmc.AeroMode(aero_data, fishy_ctor_arg) # assert - assert str(exc_info.value) == "Single-element dict expected with mode name as key and mode params dict as value" + assert ( + str(exc_info.value) + == "Single-element dict expected with mode name as key and mode params dict as value" + ) @staticmethod def test_ctor_fails_with_nonunique_mass_fracs(): @@ -280,4 +284,3 @@ def test_segfault_case(): # TODO #319 ) print(fishy_ctor_arg) ppmc.AeroMode(aero_data, fishy_ctor_arg) - From fdf7b3fc082c6463261b5d0b19b6ed911b819eb2 Mon Sep 17 00:00:00 2001 From: Sylwester Arabas Date: Sat, 2 Dec 2023 22:21:56 +0100 Subject: [PATCH 3/3] use ref to avoid copying --- src/aero_dist.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aero_dist.hpp b/src/aero_dist.hpp index a2ded63c..8bd752c9 100644 --- a/src/aero_dist.hpp +++ b/src/aero_dist.hpp @@ -51,7 +51,7 @@ struct AeroDist { { if (!InputGimmick::unique_keys(json)) throw std::runtime_error("Mode names must be unique"); - for (const auto mode : json) + for (const auto &mode : json) AeroMode::check_mode_json(mode.begin().value()); GimmickGuard guard(json, "", "mode_name", 1);