diff --git a/CHANGELOG-unreleased.md b/CHANGELOG-unreleased.md index b4390548c..a9e07480b 100644 --- a/CHANGELOG-unreleased.md +++ b/CHANGELOG-unreleased.md @@ -12,6 +12,7 @@ the released changes. - Third-order Roemer delay terms to ELL1 model - Made the addition of a TZR TOA (`AbsPhase`) in the `TimingModel` explicit in `Residuals` class. - Updated `CONTRIBUTING.rst` with the latest information. +- Made `TimingModel.params` and `TimingModel.ordered_params` identical. Deprecated `TimingModel.ordered_params`. ### Added - Third-order Roemer delay terms to ELL1 model - Options to add a TZR TOA (`AbsPhase`) during the creation of a `TimingModel` using `ModelBuilder.__call__`, `get_model`, and `get_model_and_toas` diff --git a/src/pint/fitter.py b/src/pint/fitter.py index 844d15db9..22e9860b3 100644 --- a/src/pint/fitter.py +++ b/src/pint/fitter.py @@ -363,7 +363,7 @@ def get_summary(self, nodmx=False): # to handle all parameter names, determine the longest length for the first column longestName = 0 # optionally specify the minimum length here instead of 0 - for pn in self.model.params_ordered: + for pn in self.model.params: if nodmx and pn.startswith("DMX"): continue if len(pn) > longestName: @@ -378,7 +378,7 @@ def get_summary(self, nodmx=False): s += ("{:<" + spacingName + "s} {:>20s} {:>28s} {}\n").format( "=" * longestName, "=" * 20, "=" * 28, "=" * 5 ) - for pn in self.model.params_ordered: + for pn in self.model.params: if nodmx and pn.startswith("DMX"): continue prefitpar = getattr(self.model_init, pn) diff --git a/src/pint/models/timing_model.py b/src/pint/models/timing_model.py index a7916da85..1bd05bfa2 100644 --- a/src/pint/models/timing_model.py +++ b/src/pint/models/timing_model.py @@ -183,12 +183,11 @@ class TimingModel: removed with methods on this object, and for many of them additional parameters in families (``DMXEP_1234``) can be added. - Parameters in a TimingModel object are listed in the ``model.params`` and - ``model.params_ordered`` objects. Each Parameter can be set as free or - frozen using its ``.frozen`` attribute, and a list of the free parameters - is available through the ``model.free_params`` property; this can also - be used to set which parameters are free. Several methods are available - to get and set some or all parameters in the forms of dictionaries. + Parameters in a TimingModel object are listed in the ``model.params`` object. + Each Parameter can be set as free or frozen using its ``.frozen`` attribute, + and a list of the free parameters is available through the ``model.free_params`` + property; this can also be used to set which parameters are free. Several methods + are available to get and set some or all parameters in the forms of dictionaries. TimingModel objects also support a number of functions for computing various things like orbital phase, and barycentric versions of TOAs, @@ -500,20 +499,30 @@ def __getattr__(self, name): ) @property_exists - def params(self): - """List of all parameter names in this model and all its components (order is arbitrary).""" - # FIXME: any reason not to just use params_ordered here? - p = self.top_level_params - for cp in self.components.values(): - p = p + cp.params - return p + def params_ordered(self): + """List of all parameter names in this model and all its components. + This is the same as `params`.""" + + # Historically, this was different from `params` because Python + # dictionaries were unordered until Python 3.7. Now there is no reason for + # them to be different. + + warn( + "`TimingModel.params_ordered` is now deprecated and may be removed in the future. " + "Use `TimingModel.params` instead. It gives the same output as `TimingModel.params_ordered`.", + DeprecationWarning, + ) + + return self.params @property_exists - def params_ordered(self): + def params(self): """List of all parameter names in this model and all its components, in a sensible order.""" + # Define the order of components in the list # Any not included will be printed between the first and last set. # FIXME: make order completely canonical (sort components by name?) + start_order = ["astrometry", "spindown", "dispersion"] last_order = ["jump_delay"] compdict = self.get_components_by_category() @@ -551,7 +560,7 @@ def params_ordered(self): def free_params(self): """List of all the free parameters in the timing model. Can be set to change which are free. - These are ordered as ``self.params_ordered`` does. + These are ordered as ``self.params`` does. Upon setting, order does not matter, and aliases are accepted. ValueError is raised if a parameter is not recognized. @@ -559,7 +568,7 @@ def free_params(self): On setting, parameter aliases are converted with :func:`pint.models.timing_model.TimingModel.match_param_aliases`. """ - return [p for p in self.params_ordered if not getattr(self, p).frozen] + return [p for p in self.params if not getattr(self, p).frozen] @free_params.setter def free_params(self, params): @@ -620,7 +629,7 @@ def get_params_dict(self, which="free", kind="quantity"): if which == "free": ps = self.free_params elif which == "all": - ps = self.params_ordered + ps = self.params else: raise ValueError("get_params_dict expects which to be 'all' or 'free'") c = OrderedDict() @@ -2013,10 +2022,7 @@ def compare( log.debug("Check verbosity - only warnings/info will be displayed") othermodel = copy.deepcopy(othermodel) - if ( - "POSEPOCH" in self.params_ordered - and "POSEPOCH" in othermodel.params_ordered - ): + if "POSEPOCH" in self.params and "POSEPOCH" in othermodel.params: if ( self.POSEPOCH.value is not None and othermodel.POSEPOCH.value is not None @@ -2027,7 +2033,7 @@ def compare( % (other_model_name, model_name) ) othermodel.change_posepoch(self.POSEPOCH.value) - if "PEPOCH" in self.params_ordered and "PEPOCH" in othermodel.params_ordered: + if "PEPOCH" in self.params and "PEPOCH" in othermodel.params: if ( self.PEPOCH.value is not None and self.PEPOCH.value != othermodel.PEPOCH.value @@ -2036,7 +2042,7 @@ def compare( "Updating PEPOCH in %s to match %s" % (other_model_name, model_name) ) othermodel.change_pepoch(self.PEPOCH.value) - if "DMEPOCH" in self.params_ordered and "DMEPOCH" in othermodel.params_ordered: + if "DMEPOCH" in self.params and "DMEPOCH" in othermodel.params: if ( self.DMEPOCH.value is not None and self.DMEPOCH.value != othermodel.DMEPOCH.value @@ -2071,7 +2077,7 @@ def compare( f"{model_name} is in ECL({self.ECL.value}) coordinates but {other_model_name} is in ICRS coordinates and convertcoordinates=False" ) - for pn in self.params_ordered: + for pn in self.params: par = getattr(self, pn) if par.value is None: continue @@ -2298,8 +2304,8 @@ def compare( ) # Now print any parameters in othermodel that were missing in self. - mypn = self.params_ordered - for opn in othermodel.params_ordered: + mypn = self.params + for opn in othermodel.params: if opn in mypn and getattr(self, opn).value is not None: continue if nodmx and opn.startswith("DMX"): diff --git a/tests/test_design_matrix.py b/tests/test_design_matrix.py index 0b5e06e6e..443888015 100644 --- a/tests/test_design_matrix.py +++ b/tests/test_design_matrix.py @@ -110,3 +110,8 @@ def test_combine_designmatrix_all(self): ] == 0.0 ) + + def test_param_order(self): + params_dm = self.model.designmatrix(self.toas, incoffset=False)[1] + params_free = self.model.free_params + assert params_dm == params_free