From 90c4080fb4aad25e6f180cfe35dd08333c736580 Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Tue, 22 Aug 2023 13:17:09 -0500 Subject: [PATCH 1/4] Setting no longer overrides the name of --- CHANGELOG-unreleased.md | 1 + src/pint/models/timing_model.py | 10 ++++++++++ tests/test_parameters.py | 21 +++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG-unreleased.md b/CHANGELOG-unreleased.md index 2a67299aa..d45ac7de8 100644 --- a/CHANGELOG-unreleased.md +++ b/CHANGELOG-unreleased.md @@ -25,4 +25,5 @@ the released changes. - Docstrings for `get_toas()` and `get_model_and_toas()` - Set `DelayComponent_list` and `NoiseComponent_list` to empty list if such components are absent - Fix invalid access of `PLANET_SHAPIRO` in models without `Astrometry` +- Setting `model.PARAM1 = model.PARAM2` no longer overrides the name of `PARAM1` ### Removed diff --git a/src/pint/models/timing_model.py b/src/pint/models/timing_model.py index aa73ca579..37f6d16e4 100644 --- a/src/pint/models/timing_model.py +++ b/src/pint/models/timing_model.py @@ -498,6 +498,16 @@ def __getattr__(self, name): f"Attribute {name} not found in TimingModel or any Component" ) + def __setattr__(self, name, value): + """Mostly this just sets ``self.name = value``. But in the case where they are both :class:`Parameter` instances + with different names, this copies the ``quantity``, ``uncertainty``, ``frozen`` attributes only. + """ + if isinstance(value, Parameter) and name != value.name: + for p in ["quantity", "uncertainty", "frozen"]: + setattr(getattr(self, name), p, getattr(value, p)) + else: + self.__dict__[name] = value + @property_exists def params_ordered(self): """List of all parameter names in this model and all its components. diff --git a/tests/test_parameters.py b/tests/test_parameters.py index 3a6a7b832..a91e3f206 100644 --- a/tests/test_parameters.py +++ b/tests/test_parameters.py @@ -605,3 +605,24 @@ def test_correct_number_of_params_and_FD_terms_after_add_or_remove_param(): m.remove_param("FD4") assert len(m.components["FD"].params) == 3 assert len(m.get_prefix_mapping("FD")) == 3 + + +def test_parameter_retains_name_on_set(): + basic_par_str = """ + PSR B1937+21 + LAMBDA 301.9732445337270 + BETA 42.2967523367957 + PMLAMBDA -0.0175 + PMBETA -0.3971 + PX 0.1515 + POSEPOCH 55321.0000 + F0 641.9282333345536244 1 0.0000000000000132 + F1 -4.330899370129D-14 1 2.149749089617D-22 + PEPOCH 55321.000000 + DM 71.016633 + UNITS TDB + """ + + m = get_model(io.StringIO(basic_par_str)) + m.POSEPOCH = m.PEPOCH + assert m.POSEPOCH.name == "POSEPOCH" From 65a2de9dd4aa4d433c4d3a3fd73154845b1775bf Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Tue, 22 Aug 2023 13:41:59 -0500 Subject: [PATCH 2/4] deals with prefixParameters; incompatible units fails --- src/pint/models/timing_model.py | 2 +- tests/test_parameters.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/pint/models/timing_model.py b/src/pint/models/timing_model.py index 37f6d16e4..9d0b348b8 100644 --- a/src/pint/models/timing_model.py +++ b/src/pint/models/timing_model.py @@ -502,7 +502,7 @@ def __setattr__(self, name, value): """Mostly this just sets ``self.name = value``. But in the case where they are both :class:`Parameter` instances with different names, this copies the ``quantity``, ``uncertainty``, ``frozen`` attributes only. """ - if isinstance(value, Parameter) and name != value.name: + if isinstance(value, (Parameter, prefixParameter)) and name != value.name: for p in ["quantity", "uncertainty", "frozen"]: setattr(getattr(self, name), p, getattr(value, p)) else: diff --git a/tests/test_parameters.py b/tests/test_parameters.py index a91e3f206..460bd1ca8 100644 --- a/tests/test_parameters.py +++ b/tests/test_parameters.py @@ -626,3 +626,24 @@ def test_parameter_retains_name_on_set(): m = get_model(io.StringIO(basic_par_str)) m.POSEPOCH = m.PEPOCH assert m.POSEPOCH.name == "POSEPOCH" + + +def test_parameter_set_incompatible_fails(): + basic_par_str = """ + PSR B1937+21 + LAMBDA 301.9732445337270 + BETA 42.2967523367957 + PMLAMBDA -0.0175 + PMBETA -0.3971 + PX 0.1515 + POSEPOCH 55321.0000 + F0 641.9282333345536244 1 0.0000000000000132 + F1 -4.330899370129D-14 1 2.149749089617D-22 + PEPOCH 55321.000000 + DM 71.016633 + UNITS TDB + """ + + m = get_model(io.StringIO(basic_par_str)) + with pytest.raises(u.core.UnitConversionError): + m.F0 = m.F1 From a930c2c660b3892626c814148ee4bc2019ebceb0 Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Tue, 22 Aug 2023 15:13:39 -0500 Subject: [PATCH 3/4] fixed default behavior of __setattr__ --- src/pint/models/timing_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pint/models/timing_model.py b/src/pint/models/timing_model.py index 9d0b348b8..65a013d54 100644 --- a/src/pint/models/timing_model.py +++ b/src/pint/models/timing_model.py @@ -506,7 +506,7 @@ def __setattr__(self, name, value): for p in ["quantity", "uncertainty", "frozen"]: setattr(getattr(self, name), p, getattr(value, p)) else: - self.__dict__[name] = value + super().__setattr__(name, value) @property_exists def params_ordered(self): From d8521913bc92c97ff6926c90a4bed43e58da7a7c Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Sat, 26 Aug 2023 18:14:54 -0500 Subject: [PATCH 4/4] put CHANGELOG back --- CHANGELOG-unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-unreleased.md b/CHANGELOG-unreleased.md index d7206cbe3..8b63b52d7 100644 --- a/CHANGELOG-unreleased.md +++ b/CHANGELOG-unreleased.md @@ -12,4 +12,5 @@ the released changes. ### Added ### Fixed - Fixed RTD by specifying theme explicitly. +- Setting `model.PARAM1 = model.PARAM2` no longer overrides the name of `PARAM1` ### Removed