From e3db67e5cdf94adec01e0fc4ffb553d157656fb1 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 16 Sep 2026 11:06:09 -0400 Subject: [PATCH 1/2] feat: deprecate ParameterSet camelCase Parameter accessors addParameter, newParameter and removeParameter were the public spelling of RecipeOrganizer's private _add_parameter/_new_parameter/ _remove_parameter, exposed as bare class-level aliases. Point the snake_case names at those aliases instead and add forwarding @deprecated shims for the camelCase names, matching how addParameterSet and removeParameterSet were handled in the same file. Internal callers in fitbase, pdf, sas and structure move to the new names so core no longer warns at itself. BaseSpaceGroupParameters.addParameter in structure/sgconstraints.py is deliberately untouched: it is a RecipeContainer, not a ParameterSet, so it neither inherits nor reaches these accessors, and renaming it belongs with the rest of the structure subpackage renames. This unblocks the structure and sas subpackage renames, which could not move off addParameter while it was core's only public spelling. Co-Authored-By: Claude Opus 5 --- docs/source/extending.rst | 8 +- news/addparameter-dep.rst | 26 ++++++ src/diffpy/srfit/fitbase/fitcontribution.py | 8 +- src/diffpy/srfit/fitbase/parameterset.py | 51 +++++++++++- src/diffpy/srfit/fitbase/profilegenerator.py | 6 +- src/diffpy/srfit/pdf/basepdfgenerator.py | 2 +- .../srfit/pdf/characteristicfunctions.py | 4 +- src/diffpy/srfit/pdf/pdfcontribution.py | 6 +- src/diffpy/srfit/sas/sasgenerator.py | 4 +- src/diffpy/srfit/structure/cctbxparset.py | 22 ++--- src/diffpy/srfit/structure/diffpyparset.py | 62 +++++++------- src/diffpy/srfit/structure/objcrystparset.py | 80 ++++++++++--------- tests/test_contribution.py | 8 +- tests/test_parameterset.py | 52 +++++++++++- tests/test_recipeorganizer.py | 18 ++--- tests/test_weakrefcallable.py | 2 +- 16 files changed, 240 insertions(+), 119 deletions(-) create mode 100644 news/addparameter-dep.rst diff --git a/docs/source/extending.rst b/docs/source/extending.rst index 8dc4313f..c8f01e7a 100644 --- a/docs/source/extending.rst +++ b/docs/source/extending.rst @@ -74,9 +74,9 @@ atom object called ``SimpleAtom`` that has attributes ``x``, ``y`` and ``z``. zpar = ParameterAdapter("z", atom, attr = "z") # Add these to the parameter set - self.addParameter(xpar) - self.addParameter(ypar) - self.addParameter(zpar) + self.add_parameter(xpar) + self.add_parameter(ypar) + self.add_parameter(zpar) return @@ -85,7 +85,7 @@ atom object called ``SimpleAtom`` that has attributes ``x``, ``y`` and ``z``. The ``x``, ``y`` and ``z`` attributes (specified by the ``attr`` keyword argument of ``ParameterAdapter``) of a ``SimpleAtom`` are wrapped as ``ParameterAdapter`` objects named `x`, `y`, and `z`. They are then added to -the ``SimpleAtomParSet`` using the ``addParameter`` method, which makes them +the ``SimpleAtomParSet`` using the ``add_parameter`` method, which makes them accessible as attributes. If SimpleAtom did not have an attribute named ``x``, but rather accessor diff --git a/news/addparameter-dep.rst b/news/addparameter-dep.rst new file mode 100644 index 00000000..631d9877 --- /dev/null +++ b/news/addparameter-dep.rst @@ -0,0 +1,26 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* ``ParameterSet.addParameter``, ``ParameterSet.newParameter`` and + ``ParameterSet.removeParameter`` are deprecated in favour of + ``add_parameter``, ``new_parameter`` and ``remove_parameter``. The old names + still work and warn, and will be removed in version 4.0.0. + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/srfit/fitbase/fitcontribution.py b/src/diffpy/srfit/fitbase/fitcontribution.py index d896e86e..d135205d 100644 --- a/src/diffpy/srfit/fitbase/fitcontribution.py +++ b/src/diffpy/srfit/fitbase/fitcontribution.py @@ -196,9 +196,9 @@ def set_profile(self, profile, xname=None, yname=None, dyname=None): xpar = ParameterProxy(xname, self.profile.xpar) ypar = ParameterProxy(yname, self.profile.ypar) dypar = ParameterProxy(dyname, self.profile.dypar) - self.addParameter(xpar, check=False) - self.addParameter(ypar, check=False) - self.addParameter(dypar, check=False) + self.add_parameter(xpar, check=False) + self.add_parameter(ypar, check=False) + self.add_parameter(dypar, check=False) # If we have ProfileGenerators, set their Profiles. for gen in self._generators.values(): @@ -290,7 +290,7 @@ def set_equation(self, eqstr, ns={}): ---------- eqstr : str A string representation of the equation. Any Parameter - registered by ``addParameter`` or ``set_profile``, or function + registered by ``add_parameter`` or ``set_profile``, or function registered by ``register_calculator``, ``register_function`` or ``register_string_function`` can be used in the equation by name. Other names will be turned into Parameters of this diff --git a/src/diffpy/srfit/fitbase/parameterset.py b/src/diffpy/srfit/fitbase/parameterset.py index 21bb2dac..3c031596 100644 --- a/src/diffpy/srfit/fitbase/parameterset.py +++ b/src/diffpy/srfit/fitbase/parameterset.py @@ -34,6 +34,18 @@ base, "addParameterSet", "add_parameter_set", removal_version ) +addParameter_dep_msg = build_deprecation_message( + base, "addParameter", "add_parameter", removal_version +) + +newParameter_dep_msg = build_deprecation_message( + base, "newParameter", "new_parameter", removal_version +) + +removeParameter_dep_msg = build_deprecation_message( + base, "removeParameter", "remove_parameter", removal_version +) + removeParameterSet_dep_msg = build_deprecation_message( base, "removeParameterSet", "remove_parameter_set", removal_version ) @@ -99,9 +111,42 @@ def __init__(self, name): return # Alias Parameter accessors. - addParameter = RecipeOrganizer._add_parameter - newParameter = RecipeOrganizer._new_parameter - removeParameter = RecipeOrganizer._remove_parameter + add_parameter = RecipeOrganizer._add_parameter + new_parameter = RecipeOrganizer._new_parameter + remove_parameter = RecipeOrganizer._remove_parameter + + @deprecated(addParameter_dep_msg) + def addParameter(self, parameter, check=True): + """This function has been deprecated and will be removed in version + 4.0.0. + + Please use + diffpy.srfit.fitbase.parameterset.ParameterSet.add_parameter + instead. + """ + return self.add_parameter(parameter, check) + + @deprecated(newParameter_dep_msg) + def newParameter(self, name, value, check=True): + """This function has been deprecated and will be removed in version + 4.0.0. + + Please use + diffpy.srfit.fitbase.parameterset.ParameterSet.new_parameter + instead. + """ + return self.new_parameter(name, value, check) + + @deprecated(removeParameter_dep_msg) + def removeParameter(self, parameter): + """This function has been deprecated and will be removed in version + 4.0.0. + + Please use + diffpy.srfit.fitbase.parameterset.ParameterSet.remove_parameter + instead. + """ + return self.remove_parameter(parameter) def add_parameter_set(self, parset): """Add a ParameterSet to the hierarchy. diff --git a/src/diffpy/srfit/fitbase/profilegenerator.py b/src/diffpy/srfit/fitbase/profilegenerator.py index 2b642289..2093931a 100644 --- a/src/diffpy/srfit/fitbase/profilegenerator.py +++ b/src/diffpy/srfit/fitbase/profilegenerator.py @@ -29,9 +29,9 @@ def __init__(self): # Initialize and give this a name ProfileGenerator.__init__(self, "g") # Add amplitude, center and width parameters - self.newParameter("amp", 0) - self.newParameter("center", 0) - self.newParameter("width", 0) + self.new_parameter("amp", 0) + self.new_parameter("center", 0) + self.new_parameter("width", 0) def __call__(self, x): a = self.amp.getValue() x0 = self.center.getValue() diff --git a/src/diffpy/srfit/pdf/basepdfgenerator.py b/src/diffpy/srfit/pdf/basepdfgenerator.py index 0c4d7341..6d546da8 100644 --- a/src/diffpy/srfit/pdf/basepdfgenerator.py +++ b/src/diffpy/srfit/pdf/basepdfgenerator.py @@ -122,7 +122,7 @@ def _set_calculator(self, calc): """ self._calc = calc for pname in self.__class__._parnames: - self.addParameter(ParameterAdapter(pname, self._calc, attr=pname)) + self.add_parameter(ParameterAdapter(pname, self._calc, attr=pname)) self._process_metadata() return diff --git a/src/diffpy/srfit/pdf/characteristicfunctions.py b/src/diffpy/srfit/pdf/characteristicfunctions.py index 4e760538..1acc5921 100644 --- a/src/diffpy/srfit/pdf/characteristicfunctions.py +++ b/src/diffpy/srfit/pdf/characteristicfunctions.py @@ -654,14 +654,14 @@ def __init__(self, name, model): # Wrap normal parameters for parname in model.params: par = SASParameter(parname, model) - self.addParameter(par) + self.add_parameter(par) # Wrap dispersion parameters for parname in model.dispersion: name = parname + "_width" parname += ".width" par = SASParameter(name, model, parname) - self.addParameter(par) + self.add_parameter(par) return diff --git a/src/diffpy/srfit/pdf/pdfcontribution.py b/src/diffpy/srfit/pdf/pdfcontribution.py index b103a414..dc54adb3 100644 --- a/src/diffpy/srfit/pdf/pdfcontribution.py +++ b/src/diffpy/srfit/pdf/pdfcontribution.py @@ -96,10 +96,10 @@ def __init__(self, name): # Need a parameter for the overall scale, in the case that this is a # multi-phase fit. - self.newParameter("scale", 1.0) + self.new_parameter("scale", 1.0) # Profile-related parameters that will be shared between the generators - self.newParameter("qdamp", 0) - self.newParameter("qbroad", 0) + self.new_parameter("qdamp", 0) + self.new_parameter("qbroad", 0) return # Data methods diff --git a/src/diffpy/srfit/sas/sasgenerator.py b/src/diffpy/srfit/sas/sasgenerator.py index a986e283..df3024aa 100644 --- a/src/diffpy/srfit/sas/sasgenerator.py +++ b/src/diffpy/srfit/sas/sasgenerator.py @@ -57,14 +57,14 @@ def __init__(self, name, model): # Wrap normal parameters for parname in model.params: par = SASParameter(parname, model) - self.addParameter(par) + self.add_parameter(par) # Wrap dispersion parameters for parname in model.dispersion: name = parname + "_width" parname += ".width" par = SASParameter(name, model, parname) - self.addParameter(par) + self.add_parameter(par) return diff --git a/src/diffpy/srfit/structure/cctbxparset.py b/src/diffpy/srfit/structure/cctbxparset.py index 99711c07..d2c89023 100644 --- a/src/diffpy/srfit/structure/cctbxparset.py +++ b/src/diffpy/srfit/structure/cctbxparset.py @@ -70,19 +70,19 @@ def __init__(self, name, strups, idx): self.idx = idx # x, y, z, occupancy - self.addParameter( + self.add_parameter( ParameterAdapter("x", None, self._xyzgetter(0), self._xyzsetter(0)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("y", None, self._xyzgetter(1), self._xyzsetter(1)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("z", None, self._xyzgetter(2), self._xyzsetter(2)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("occupancy", None, self._getocc, self._setocc) ) - self.addParameter( + self.add_parameter( ParameterAdapter("Uiso", None, self._getuiso, self._setuiso) ) return @@ -163,26 +163,26 @@ def __init__(self, strups): self.strups = strups self._latpars = list(self.strups.stru.unit_cell().parameters()) - self.addParameter( + self.add_parameter( ParameterAdapter("a", None, self._latgetter(0), self._latsetter(0)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("b", None, self._latgetter(1), self._latsetter(1)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("c", None, self._latgetter(2), self._latsetter(2)) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "alpha", None, self._latgetter(3), self._latsetter(3) ) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "beta", None, self._latgetter(4), self._latsetter(4) ) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "gamma", None, self._latgetter(5), self._latsetter(5) ) diff --git a/src/diffpy/srfit/structure/diffpyparset.py b/src/diffpy/srfit/structure/diffpyparset.py index 545cb976..6d2d17b6 100644 --- a/src/diffpy/srfit/structure/diffpyparset.py +++ b/src/diffpy/srfit/structure/diffpyparset.py @@ -102,52 +102,52 @@ def __init__(self, name, atom): self.atom = atom a = atom # x, y, z, occupancy - self.addParameter( + self.add_parameter( ParameterAdapter("x", a, _xyzgetter(0), _xyzsetter(0)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("y", a, _xyzgetter(1), _xyzsetter(1)) ) - self.addParameter( + self.add_parameter( ParameterAdapter("z", a, _xyzgetter(2), _xyzsetter(2)) ) occupancy = ParameterAdapter("occupancy", a, attr="occupancy") - self.addParameter(occupancy) - self.addParameter(ParameterProxy("occ", occupancy)) + self.add_parameter(occupancy) + self.add_parameter(ParameterProxy("occ", occupancy)) # U - self.addParameter(ParameterAdapter("U11", a, attr="U11")) - self.addParameter(ParameterAdapter("U22", a, attr="U22")) - self.addParameter(ParameterAdapter("U33", a, attr="U33")) + self.add_parameter(ParameterAdapter("U11", a, attr="U11")) + self.add_parameter(ParameterAdapter("U22", a, attr="U22")) + self.add_parameter(ParameterAdapter("U33", a, attr="U33")) U12 = ParameterAdapter("U12", a, attr="U12") U21 = ParameterProxy("U21", U12) U13 = ParameterAdapter("U13", a, attr="U13") U31 = ParameterProxy("U31", U13) U23 = ParameterAdapter("U23", a, attr="U23") U32 = ParameterProxy("U32", U23) - self.addParameter(U12) - self.addParameter(U21) - self.addParameter(U13) - self.addParameter(U31) - self.addParameter(U23) - self.addParameter(U32) - self.addParameter(ParameterAdapter("Uiso", a, attr="Uisoequiv")) + self.add_parameter(U12) + self.add_parameter(U21) + self.add_parameter(U13) + self.add_parameter(U31) + self.add_parameter(U23) + self.add_parameter(U32) + self.add_parameter(ParameterAdapter("Uiso", a, attr="Uisoequiv")) # B - self.addParameter(ParameterAdapter("B11", a, attr="B11")) - self.addParameter(ParameterAdapter("B22", a, attr="B22")) - self.addParameter(ParameterAdapter("B33", a, attr="B33")) + self.add_parameter(ParameterAdapter("B11", a, attr="B11")) + self.add_parameter(ParameterAdapter("B22", a, attr="B22")) + self.add_parameter(ParameterAdapter("B33", a, attr="B33")) B12 = ParameterAdapter("B12", a, attr="B12") B21 = ParameterProxy("B21", B12) B13 = ParameterAdapter("B13", a, attr="B13") B31 = ParameterProxy("B31", B13) B23 = ParameterAdapter("B23", a, attr="B23") B32 = ParameterProxy("B32", B23) - self.addParameter(B12) - self.addParameter(B21) - self.addParameter(B13) - self.addParameter(B31) - self.addParameter(B23) - self.addParameter(B32) - self.addParameter(ParameterAdapter("Biso", a, attr="Bisoequiv")) + self.add_parameter(B12) + self.add_parameter(B21) + self.add_parameter(B13) + self.add_parameter(B31) + self.add_parameter(B23) + self.add_parameter(B32) + self.add_parameter(ParameterAdapter("Biso", a, attr="Bisoequiv")) return def __repr__(self): @@ -216,26 +216,26 @@ def __init__(self, lattice): self.angunits = "deg" self.lattice = lattice lat = lattice - self.addParameter( + self.add_parameter( ParameterAdapter("a", lat, _latgetter("a"), _latsetter("a")) ) - self.addParameter( + self.add_parameter( ParameterAdapter("b", lat, _latgetter("b"), _latsetter("b")) ) - self.addParameter( + self.add_parameter( ParameterAdapter("c", lat, _latgetter("c"), _latsetter("c")) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "alpha", lat, _latgetter("alpha"), _latsetter("alpha") ) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "beta", lat, _latgetter("beta"), _latsetter("beta") ) ) - self.addParameter( + self.add_parameter( ParameterAdapter( "gamma", lat, _latgetter("gamma"), _latsetter("gamma") ) diff --git a/src/diffpy/srfit/structure/objcrystparset.py b/src/diffpy/srfit/structure/objcrystparset.py index 755f818e..22e9ec08 100644 --- a/src/diffpy/srfit/structure/objcrystparset.py +++ b/src/diffpy/srfit/structure/objcrystparset.py @@ -130,10 +130,12 @@ def __init__(self, name, scat, parent): self.parent = parent # x, y, z, occ - self.addParameter(ParameterAdapter("x", self.scat, attr="X")) - self.addParameter(ParameterAdapter("y", self.scat, attr="Y")) - self.addParameter(ParameterAdapter("z", self.scat, attr="Z")) - self.addParameter(ParameterAdapter("occ", self.scat, attr="Occupancy")) + self.add_parameter(ParameterAdapter("x", self.scat, attr="X")) + self.add_parameter(ParameterAdapter("y", self.scat, attr="Y")) + self.add_parameter(ParameterAdapter("z", self.scat, attr="Z")) + self.add_parameter( + ParameterAdapter("occ", self.scat, attr="Occupancy") + ) return def isDummy(self): @@ -191,22 +193,22 @@ def __init__(self, name, atom, parent): sp = atom.GetScatteringPower() # The B-parameters - self.addParameter(ParameterAdapter("Biso", sp, attr="Biso")) - self.addParameter(ParameterAdapter("B11", sp, attr="B11")) - self.addParameter(ParameterAdapter("B22", sp, attr="B22")) - self.addParameter(ParameterAdapter("B33", sp, attr="B33")) + self.add_parameter(ParameterAdapter("Biso", sp, attr="Biso")) + self.add_parameter(ParameterAdapter("B11", sp, attr="B11")) + self.add_parameter(ParameterAdapter("B22", sp, attr="B22")) + self.add_parameter(ParameterAdapter("B33", sp, attr="B33")) B12 = ParameterAdapter("B12", sp, attr="B12") B21 = ParameterProxy("B21", B12) B13 = ParameterAdapter("B13", sp, attr="B13") B31 = ParameterProxy("B31", B13) B23 = ParameterAdapter("B23", sp, attr="B23") B32 = ParameterProxy("B32", B23) - self.addParameter(B12) - self.addParameter(B21) - self.addParameter(B13) - self.addParameter(B31) - self.addParameter(B23) - self.addParameter(B32) + self.add_parameter(B12) + self.add_parameter(B21) + self.add_parameter(B13) + self.add_parameter(B31) + self.add_parameter(B23) + self.add_parameter(B32) # Give a value to Biso if it doesn't have one, and this is isotropic if sp.IsIsotropic() and self.Biso.value == 0: @@ -267,10 +269,10 @@ def __init__(self, name, molecule, parent=None): self.stru = molecule # Add orientation quaternion - self.addParameter(ParameterAdapter("q0", self.scat, attr="Q0")) - self.addParameter(ParameterAdapter("q1", self.scat, attr="Q1")) - self.addParameter(ParameterAdapter("q2", self.scat, attr="Q2")) - self.addParameter(ParameterAdapter("q3", self.scat, attr="Q3")) + self.add_parameter(ParameterAdapter("q0", self.scat, attr="Q0")) + self.add_parameter(ParameterAdapter("q1", self.scat, attr="Q1")) + self.add_parameter(ParameterAdapter("q2", self.scat, attr="Q2")) + self.add_parameter(ParameterAdapter("q3", self.scat, attr="Q3")) # Wrap the MolAtoms within the molecule self.atoms = [] @@ -400,7 +402,7 @@ def wrapStretchModeParameters(self): par.AddAtoms(atoms) - self.addParameter(par) + self.add_parameter(par) for mode in self.scat.GetStretchModeBondAngleList(): name1 = mode.mpAtom0.GetName() @@ -423,7 +425,7 @@ def wrapStretchModeParameters(self): atoms.append(getattr(self, name)) par.AddAtoms(atoms) - self.addParameter(par) + self.add_parameter(par) return @@ -686,7 +688,7 @@ def addBondLengthParameter( Returns the new ObjCrystBondLengthParameter. """ par = ObjCrystBondLengthParameter(name, atom1, atom2, value, const) - self.addParameter(par) + self.add_parameter(par) return par @@ -726,7 +728,7 @@ def addBondAngleParameter( par = ObjCrystBondAngleParameter( name, atom1, atom2, atom3, value, const ) - self.addParameter(par) + self.add_parameter(par) return par @@ -770,7 +772,7 @@ def addDihedralAngleParameter( par = ObjCrystDihedralAngleParameter( name, atom1, atom2, atom3, atom4, value, const ) - self.addParameter(par) + self.add_parameter(par) return par @@ -825,22 +827,22 @@ def __init__(self, name, scat, parent): # Only wrap this if there is a scattering power if sp is not None: - self.addParameter(ParameterAdapter("Biso", sp, attr="Biso")) - self.addParameter(ParameterAdapter("B11", sp, attr="B11")) - self.addParameter(ParameterAdapter("B22", sp, attr="B22")) - self.addParameter(ParameterAdapter("B33", sp, attr="B33")) + self.add_parameter(ParameterAdapter("Biso", sp, attr="Biso")) + self.add_parameter(ParameterAdapter("B11", sp, attr="B11")) + self.add_parameter(ParameterAdapter("B22", sp, attr="B22")) + self.add_parameter(ParameterAdapter("B33", sp, attr="B33")) B12 = ParameterAdapter("B12", sp, attr="B12") B21 = ParameterProxy("B21", B12) B13 = ParameterAdapter("B13", sp, attr="B13") B31 = ParameterProxy("B31", B13) B23 = ParameterAdapter("B23", sp, attr="B23") B32 = ParameterProxy("B32", B23) - self.addParameter(B12) - self.addParameter(B21) - self.addParameter(B13) - self.addParameter(B31) - self.addParameter(B23) - self.addParameter(B32) + self.add_parameter(B12) + self.add_parameter(B21) + self.add_parameter(B13) + self.add_parameter(B31) + self.add_parameter(B23) + self.add_parameter(B32) return @@ -1817,12 +1819,12 @@ def __init__(self, name, cryst): self.stru = cryst self._sgpars = None - self.addParameter(ParameterAdapter("a", self.stru, attr="a")) - self.addParameter(ParameterAdapter("b", self.stru, attr="b")) - self.addParameter(ParameterAdapter("c", self.stru, attr="c")) - self.addParameter(ParameterAdapter("alpha", self.stru, attr="alpha")) - self.addParameter(ParameterAdapter("beta", self.stru, attr="beta")) - self.addParameter(ParameterAdapter("gamma", self.stru, attr="gamma")) + self.add_parameter(ParameterAdapter("a", self.stru, attr="a")) + self.add_parameter(ParameterAdapter("b", self.stru, attr="b")) + self.add_parameter(ParameterAdapter("c", self.stru, attr="c")) + self.add_parameter(ParameterAdapter("alpha", self.stru, attr="alpha")) + self.add_parameter(ParameterAdapter("beta", self.stru, attr="beta")) + self.add_parameter(ParameterAdapter("gamma", self.stru, attr="gamma")) # Now we must loop over the scatterers and create parameter sets from # them. diff --git a/tests/test_contribution.py b/tests/test_contribution.py index 56975905..6c2f3059 100644 --- a/tests/test_contribution.py +++ b/tests/test_contribution.py @@ -303,9 +303,9 @@ def test_setEquation(noObserversInGlobalBuilders): fc.setEquation("x + 5") fc.x.set_value(2) assert 7 == fc.evaluate() - fc.removeParameter(fc.x) + fc.remove_parameter(fc.x) x = arange(0, 10, 0.5) - fc.newParameter("x", x) + fc.new_parameter("x", x) assert np.array_equal(5 + x, fc.evaluate()) assert noObserversInGlobalBuilders return @@ -317,9 +317,9 @@ def test_set_equation(noObserversInGlobalBuilders): fc.set_equation("x + 5") fc.x.set_value(2) assert 7 == fc.evaluate() - fc.removeParameter(fc.x) + fc.remove_parameter(fc.x) x = arange(0, 10, 0.5) - fc.newParameter("x", x) + fc.new_parameter("x", x) assert np.array_equal(5 + x, fc.evaluate()) assert noObserversInGlobalBuilders return diff --git a/tests/test_parameterset.py b/tests/test_parameterset.py index 1838c2f4..ea94d266 100644 --- a/tests/test_parameterset.py +++ b/tests/test_parameterset.py @@ -14,8 +14,11 @@ ############################################################################## """Tests for refinableobj module.""" +import re import unittest +import pytest + from diffpy.srfit.fitbase.parameter import Parameter from diffpy.srfit.fitbase.parameterset import ParameterSet @@ -37,7 +40,7 @@ def test_add_parameter_set(self): self.assertRaises(ValueError, self.parset.add_parameter_set, p1) p1.name = "p1" - parset2.addParameter(p1) + parset2.add_parameter(p1) self.assertTrue(self.parset.parset2.p1 is p1) @@ -70,12 +73,57 @@ def test_add_parameter_set_deprecated(self): self.assertRaises(ValueError, self.parset.add_parameter_set, p1) p1.name = "p1" - parset2.addParameter(p1) + parset2.add_parameter(p1) self.assertTrue(self.parset.parset2.p1 is p1) return +# ---------------------------------------------------------------------------- +# The camelCase Parameter accessors on ParameterSet are deprecated in favour of +# their snake_case spellings. Each old name must still work, warn with a +# message that names its replacement, and have the same effect on the set. + + +@pytest.mark.parametrize( + "deprecated_name, replacement_name", + [ + # C1: A Parameter is stored under the set. + # Expected: addParameter warns and stores it as add_parameter does. + ("addParameter", "add_parameter"), + # C2: A Parameter is created and stored in one call. + # Expected: newParameter warns and creates it as new_parameter does. + ("newParameter", "new_parameter"), + # C3: A stored Parameter is removed from the set. + # Expected: removeParameter warns and removes it as + # remove_parameter does. + ("removeParameter", "remove_parameter"), + ], +) +def test_parameter_accessors_warn_and_forward( + deprecated_name, replacement_name +): + base = "diffpy.srfit.fitbase.parameterset.ParameterSet" + expected_msg = ( + f"'{base}.{deprecated_name}' is deprecated and will be removed in " + f"version 4.0.0. Please use '{base}.{replacement_name}' instead." + ) + parset = ParameterSet("test") + if deprecated_name == "newParameter": + arguments = ("p1", 1) + elif deprecated_name == "removeParameter": + arguments = (parset.new_parameter("p1", 1),) + else: + arguments = (Parameter("p1", 1),) + + with pytest.warns(DeprecationWarning, match=re.escape(expected_msg)): + getattr(parset, deprecated_name)(*arguments) + + expected_names = [] if deprecated_name == "removeParameter" else ["p1"] + actual_names = [par.name for par in parset._parameters.values()] + assert actual_names == expected_names + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_recipeorganizer.py b/tests/test_recipeorganizer.py index 1b953c3b..193b35ed 100644 --- a/tests/test_recipeorganizer.py +++ b/tests/test_recipeorganizer.py @@ -190,7 +190,7 @@ def tearDown(self): return def testNewParameter(self): - """Test the addParameter method.""" + """Test the add_parameter method.""" m = self.m p1 = Parameter("p1", 1) @@ -205,7 +205,7 @@ def testNewParameter(self): return def testAddParameter(self): - """Test the addParameter method.""" + """Test the add_parameter method.""" m = self.m p1 = Parameter("p1", 1) @@ -242,7 +242,7 @@ def testAddParameter(self): return def testRemoveParameter(self): - """Test removeParameter method.""" + """Test remove_parameter method.""" m = self.m p1 = Parameter("p1", 1) @@ -419,9 +419,9 @@ class GCalc(Calculator): def __init__(self, name): Calculator.__init__(self, name) - self.newParameter("A", 1.0) - self.newParameter("center", 0.0) - self.newParameter("width", 0.1) + self.new_parameter("A", 1.0) + self.new_parameter("center", 0.0) + self.new_parameter("width", 0.1) return def __call__(self, x): @@ -464,9 +464,9 @@ class GCalc(Calculator): def __init__(self, name): Calculator.__init__(self, name) - self.newParameter("A", 1.0) - self.newParameter("center", 0.0) - self.newParameter("width", 0.1) + self.new_parameter("A", 1.0) + self.new_parameter("center", 0.0) + self.new_parameter("width", 0.1) return def __call__(self, x): diff --git a/tests/test_weakrefcallable.py b/tests/test_weakrefcallable.py index 688bc563..e6d58fa6 100644 --- a/tests/test_weakrefcallable.py +++ b/tests/test_weakrefcallable.py @@ -110,7 +110,7 @@ def test_pickling(self): def test_observable_deregistration(self): """Check if Observable drops dead Observer.""" f = self.f - x = f.newParameter("x", 5) + x = f.new_parameter("x", 5) f.set_equation("3 * x") self.assertEqual(15, f.evaluate()) self.assertEqual(15, f._eq._value) From caca7257d3078a164eef2ca53d53294e50153bcd Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Wed, 16 Sep 2026 12:55:46 -0400 Subject: [PATCH 2/2] change test format --- tests/test_parameterset.py | 80 ++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/tests/test_parameterset.py b/tests/test_parameterset.py index ea94d266..a1e15a30 100644 --- a/tests/test_parameterset.py +++ b/tests/test_parameterset.py @@ -86,41 +86,63 @@ def test_add_parameter_set_deprecated(self): # message that names its replacement, and have the same effect on the set. -@pytest.mark.parametrize( - "deprecated_name, replacement_name", - [ - # C1: A Parameter is stored under the set. - # Expected: addParameter warns and stores it as add_parameter does. - ("addParameter", "add_parameter"), - # C2: A Parameter is created and stored in one call. - # Expected: newParameter warns and creates it as new_parameter does. - ("newParameter", "new_parameter"), - # C3: A stored Parameter is removed from the set. - # Expected: removeParameter warns and removes it as - # remove_parameter does. - ("removeParameter", "remove_parameter"), - ], -) -def test_parameter_accessors_warn_and_forward( - deprecated_name, replacement_name -): - base = "diffpy.srfit.fitbase.parameterset.ParameterSet" +# C1: A Parameter is stored under the set with the deprecated name. +# Expected: addParameter warns and stores it as add_parameter does. +def test_add_parameter_deprecated(): expected_msg = ( - f"'{base}.{deprecated_name}' is deprecated and will be removed in " - f"version 4.0.0. Please use '{base}.{replacement_name}' instead." + "'diffpy.srfit.fitbase.parameterset.ParameterSet.addParameter' is " + "deprecated and will be removed in version 4.0.0. " + "Please use " + "'diffpy.srfit.fitbase.parameterset.ParameterSet.add_parameter' " + "instead." ) + expected_names = ["p1"] parset = ParameterSet("test") - if deprecated_name == "newParameter": - arguments = ("p1", 1) - elif deprecated_name == "removeParameter": - arguments = (parset.new_parameter("p1", 1),) - else: - arguments = (Parameter("p1", 1),) with pytest.warns(DeprecationWarning, match=re.escape(expected_msg)): - getattr(parset, deprecated_name)(*arguments) + parset.addParameter(Parameter("p1", 1)) + + actual_names = [par.name for par in parset._parameters.values()] + assert actual_names == expected_names + + +# C2: A Parameter is created and stored with the deprecated name. +# Expected: newParameter warns and creates it as new_parameter does. +def test_new_parameter_deprecated(): + expected_msg = ( + "'diffpy.srfit.fitbase.parameterset.ParameterSet.newParameter' is " + "deprecated and will be removed in version 4.0.0. " + "Please use " + "'diffpy.srfit.fitbase.parameterset.ParameterSet.new_parameter' " + "instead." + ) + expected_names = ["p1"] + parset = ParameterSet("test") + + with pytest.warns(DeprecationWarning, match=re.escape(expected_msg)): + parset.newParameter("p1", 1) + + actual_names = [par.name for par in parset._parameters.values()] + assert actual_names == expected_names + + +# C3: A stored Parameter is removed from the set with the deprecated name. +# Expected: removeParameter warns and removes it as remove_parameter does. +def test_remove_parameter_deprecated(): + expected_msg = ( + "'diffpy.srfit.fitbase.parameterset.ParameterSet.removeParameter' is " + "deprecated and will be removed in version 4.0.0. " + "Please use " + "'diffpy.srfit.fitbase.parameterset.ParameterSet.remove_parameter' " + "instead." + ) + expected_names = [] + parset = ParameterSet("test") + p1 = parset.new_parameter("p1", 1) + + with pytest.warns(DeprecationWarning, match=re.escape(expected_msg)): + parset.removeParameter(p1) - expected_names = [] if deprecated_name == "removeParameter" else ["p1"] actual_names = [par.name for par in parset._parameters.values()] assert actual_names == expected_names