Deprecated: Fitcontribution camel case methods

[cadenmyers13]

6 methods of FitContribution are deprecated here. See commit messages for which. I pytest passed locally and fitbulkNi works (and throws Deprecation message).

[cadenmyers13]

@sbillinge ready for review

[cadenmyers13]

@sbillinge jk... we need a release of utils first.

/usr/share/miniconda/envs/test/lib/python3.13/site-packages/diffpy/srfit/fitbase/fitcontribution.py:32: in <module>
    from diffpy.utils._deprecator import deprecated
E   ModuleNotFoundError: No module named 'diffpy.utils._deprecator'

[sbillinge]

this looks fantastic. Please see my comments though. These are important to get right before we farm this out to workers.

I suggest that we duplicate at least one of the tests for the old filename. We don't have to duplicate all the tests with the old name. This just tests that our deprecation hasn't broken the use of the old name. If one one test passes with the old name, and all the tests pass with the new name, we should be ok (preferrably it would be a test that uses all the available arguments though, so don't pick a trivial test but a rich one.

[sbillinge]

please can you list all hte public facing methods that were depracated as a matter of policy. This is user facing so it is more useful to them than which module from a dev point of view that you worked on.

[sbillinge]

Let's make sure we have an issue to remove this from conda.txt and pip.txt when the old API is removed.

[cadenmyers13]

done

[sbillinge]

For greater readability lets make version to be removal_version. Also, I don't see any default behavior, so let's make it required and not optional.

But this could get a bit messy. I had a quick convo with chatgpt about this. It is slightly messy as I am sure you have found out, but defining this message in a central place can work We have to have the function here also, but it is more lightweight. Here is a summary:
Yes — in this case you can use __name__, but only if you evaluate it in the module where the message is constructed, not inside the imported helper function.

The key distinction is:

• __name__ is resolved lexically, at function definition time, not at import time or call time.
• Therefore, if you place _dep_msg_fitcontrib in a shared helper module, the value of __name__ inside that function will always be the helper module’s name, not the name of the module that imported it.

So you cannot simply write:

# shared_helpers.py
def _dep_msg_fitcontrib(...):
    print(__name__)   # always "shared_helpers", never "diffpy.srfit.fitbase..."

---

The part that is easy

If what you really want is to avoid repeating the long module path in the string:

diffpy.srfit.fitbase.fitcontribution.FitContribution

and make each module auto-insert its own module path, the simplest and correct approach is to construct the prefix in the calling module, not inside the helper.

Example: Best pattern using __name__

# in shared_helpers.py
def make_deprecation_message(prefix, old_name, new_name, version):
    return (
        f"'{prefix}.{old_name}' is deprecated and will be removed"
        f" in version {version}. Please use '{prefix}.{new_name}' instead."
    )

Then in each module:

# diffpy/srfit/fitbase/fitcontribution.py

from shared_helpers import make_deprecation_message

_PREFIX = __name__ + ".FitContribution"
_REMOVAL_VERSION = 4.0.0

def _dep_msg_fitcontrib(old_name, new_name):
    return make_deprecation_message(_PREFIX, old_name, new_name, _REMOVAL_VERSION)

However, I would also centralize the definition of removal_version (though make it reusable so we can reuse this in future updates.

[cadenmyers13]

@sbillinge okay i changed some things a bit. First off (as recommended) I changed one test for setProfile to have the old version. This tests passes and pytest also throws the deprecation warning message which is great. see screeshot:

Secondly, I added the deprecation message to diffpy.utils in this PR diffpy/diffpy.utils#367. The process for deprecation is now,

1. In top level __init__.py, define a _REMOVAL_VERSION variable which is the version where the deprecated function will be removed
2. Go to the module where deprecation will take place and import the removal version, and deprecation tools. Also, define a base variable that references what will be removed. ex,

from diffpy.srfit import _REMOVAL_VERSION
from diffpy.utils._deprecator import deprecated, deprecation_message

_BASE_FITCONTRIBUTION = __name__ + ".FitContribution"

3. Use @deprecated to deprecate a method/function/etc,

    @deprecated(
        deprecation_message(
            _BASE_FITCONTRIBUTION,
            "setProfile",
            "set_profile",
            _REMOVAL_VERSION,
        )
    )
    def setProfile(self, profile, xname=None, yname=None, dyname=None):

Once again, I tested this with fitBulkNi.py and it worked. see screenshot:

[sbillinge]

@cadenmyers13 this is great. It is failing tests but I will merge when it gets going again.

[sbillinge]

also, just to check, hopefully you didn't change one test from the new name to the old name, but duplicated one of the tests with the old name, so when we delete it we are not losing a test. Please confirm, I wasn't sure from your comment.

[cadenmyers13]

@sbillinge awesome, its failing tests on CI because we're importing something from utils that isn't released yet. All tests pass locally. Once we do a release of utils it should pass

[cadenmyers13]

also, just to check, hopefully you didn't change one test from the new name to the old name, but duplicated one of the tests with the old name, so when we delete it we are not losing a test. Please confirm, I wasn't sure from your comment.

@sbillinge Oops I did not duplicate. I'll duplicate it

[cadenmyers13]

@sbillinge okay, i duplicated it. Both versions passed locally

[sbillinge]

@cadenmyers13 We need a new release of diffpy.utils for this to pass CI. We could push out a hot-fix patch release but I am reluctant to do this. It will be a bunch of work to avoid mixing it up with the next major release. It can be done but at some cost in time.

Alternatively we can work to get the next minor release (3.7.0) out. This is the preferred approach. You can see what the remaining issues are on the milestone. Could you take a look and see if we can get this done? It is related to the diffpy.labpdfproc release that I mentioned on slack also.

[cadenmyers13]

Could you take a look and see if we can get this done?

@sbillinge Looks like its all aesthetic stuff (docs and such) with one thing that needs to be refactored diffpy/diffpy.utils#358. Shouldn't be too difficult, maybe more tedious than anything