DEPR: self-triggered deprecation warnings in 3.2.4

[neutrinoceros]

ref: #336

If I understand correctly, UFloat.error_components() is equivalent to UFloat.derivatives. 3.2.4 deprecated both, and unfortunately the migration path isn't straight forward:

• there is no way to write code that will work with both uncertainties 3.x and 4.x. As a result, the only way to protect users of a library that already depends on uncertainties against breaking upgrades is to introduce an upper bound <4 on the requirement, whith is an infamous antipattern
• even if I set an upper bound and restrict my own usage to UFloat.error_components() for now, I'll still get FutureWarnings from it because it is wired to use UFloat.derivatives internally. I can ignore the warnings in my own CI, but cannot protect my users against it without resorting to an even worse upper bound (<3.2.4).

One way to fix both problems, would be to instead deprecate the error_components method in favor of the existing derivatives property.
I would also advise yanking 3.2.4 for good measure.

[newville]

@neutrinoceros As you might have gathered from #336, our understanding is that not many people are using these methods. I think we would be interested to hear about your

Yes, 4.0 will be backward incompatible with 3.x. That is common for changes in major versions. At #336, we discussed the consequences of converting error_components from a method to a property for 4.0, and decided this was worth doing. The idea was to deprecate that in 3.2.4, and then remove it for 4.0.

And, yes, 3.2.4 gives too many deprecation warnings. I don't fully agree that extra deprecation warnings warrant yanking a release, but I do agree we should release 3.2.5 right away.

[neutrinoceros]

I think we would be interested to hear about your

I help maintain the AMICAL library, which uses uncertainties.ufloat internally. For instance
https://github.com/SAIL-Labs/AMICAL/blob/ac809fdf3577e8bb81198b867b3ea766b2b89420/src/amical/analysis/easy_candid.py#L52C31-L52C37

Yes, 4.0 will be backward incompatible with 3.x. That is common for changes in major versions

The problem isn't that you're planning a backward incompatible change, especially if you're doing it as a major version. The issue is specifically that there is no way to adapt code to work with both versions, while such a solution does exist. Providing a clear (albeit narrow) backward and forward compatible way to rewrite downstream code is a highly desirable property in deprecation cycles. I would also note that, in contrast to shipping backward incompatible changes in a major release, adding deprecations in a patch release is not common; any CI that treats warnings as errors (as AMICAL's does, but more importantly as I believe is the correct thing to do when maintaining libraries) will choke on this upgrade.

[neutrinoceros]

I don't fully agree that extra deprecation warnings warrant yanking a release, but I do agree we should release 3.2.5 right away.

yanking isn't critical, especially if you're on board with doing a 3.2.5 sooner rather than later :)

[jagerber48]

@newville I see your 3.2.5a, I have some issues with it but not a lot of time to go into details right now. I don't have a lot of time to dedicate to uncertainties in general right now. I think yanking 3.2.4 is the way to go while we take time figuring out the right path forward. There's nothing special in 3.2.4 that I will regret being yanked. It has a bad behavior we did not want and I think @neutrinoceros concern about no migration path should be more carefully considered. @wshanks also brought it up in another thread. We decided it was ok, but I think it's worth taking this new community feedback into account.

[newville]

@jagerber48 Please approve #353, then all the other changes and fixes for 3.2.4 are accepted, and the warnings become PendingDeprecationWarnings.

[jagerber48]

@neutrinoceros

there is no way to write code that will work with both uncertainties 3.x and 4.x. As a result, the only way to protect users of a library that already depends on uncertainties against breaking upgrades is to introduce an upper bound <4 on the requirement, whith is an infamous antipattern

This is exactly correct.

We wanted to rename error_components() (an instance method) to error_components, an instance method because we think it semantically makes more sense that way.

The problem isn't that you're planning a backward incompatible change, especially if you're doing it as a major version. The issue is specifically that there is no way to adapt code to work with both versions, while such a solution does exist. Providing a clear (albeit narrow) backward and forward compatible way to rewrite downstream code is a highly desirable property in deprecation cycles.

Unfortunately we weren't able to find such a path for converting a python instance method (error_components()) into an instance property error_components. The issue is there is a name collision between the two thing and, to my knowledge, there isn't some sort of overloading we can do in python to support both such that we could have even one version of cross compatibility AND satisfying the requirement that we always return an ordinary python dict to the user. I am all ears if you have any proposals about how we could do it.

I would also note that, in contrast to shipping backward incompatible changes in a major release, adding deprecations in a patch release is not common; any CI that treats warnings as errors (as AMICAL's does, but more importantly as I believe is the correct thing to do when maintaining libraries) will choke on this upgrade.

Thanks for the suggestion about not adding deprecations in a patch release. We can consider doing it in feature/minor release. I now have a goal to make uncertainties CI fail if it encounters a warning like this. I'm frustrated our CI let this issue through.

---

@neutrinoceros does AMICAL call error_components() directly? Or are you just encountering this error through the ufloat interface? If it is the former then this issue is essentially a duplicate of #350. I did NOT intend for users to get FutureWarning just for calling ufloat(1, 1) + ufloat(1, 1). The intention was for users EXPLICITLY calling error_components() to get a warning. However, INTERNAL usages of error_components() are also raising the warning. In my opinion we should have guarded internal usages with warnings filtering context managers so that the warnings don't propagate up to the user. This is why I think 3.2.4 is BROKEN and we should yank it.

@neutrinoceros if you DO NOT call error_components() directly then you would not have been impacted by the intended behavior of this deprecation. In fact, we did some brief usage research and found that hardly anyone is calling error_components() directly. That helped us justify the decision to pursue the abrupt breaking change with no "nice" migration path.

[Littie28]

Following up on @neutrinoceros's point about internal usages raising the warning - I ran into this through lmfit and put together a fix.

The approach: split into internal _-prefixed versions that don't warn, keeping the public API with the deprecation warning.

@property
def _derivatives(self):
    """Internal access - no warning."""
    if not self._linear_part.expanded():
        self._linear_part.expand()
        self._linear_part.linear_combo.default_factory = None
    return self._linear_part.linear_combo

@property
def derivatives(self):
    """Public API - warns on access."""
    warn(
        f"{self.__class__.__name__}.derivatives() is deprecated. It will "
        f"be removed in a future release.",
        FutureWarning,
        stacklevel=2,
    )
    return self._derivatives

Same pattern for error_components. All internal calls now use the _-prefixed versions.

Test:

from uncertainties import ufloat

x = ufloat(1, 0.1)
y = ufloat(2, 0.2)
z = x + y

print(z.std_dev)       # No warning (internal access)
print(z.derivatives)   # Warns (public deprecated API)

0.223606797749979
FutureWarning: AffineScalarFunc.derivatives() is deprecated...

Happy to open a PR if this approach looks good.

[neutrinoceros]

I now have a goal to make uncertainties CI fail if it encounters a warning like this.

treating warnings as errors in CI is how I found out about this deprecation and would indeed also be advisable in uncertainties as it would prevent such mishaps to happen in the future. Thanks for your consideration !

does AMICAL call error_components() directly?

thanks for asking. Indeed I failed to recognise that AMICAL didn't even call this method directly. What it's actually calling is AffineScalarFunc.std_dev, which in turns calls its error_component method internally.

In my opinion we should have guarded internal usages with warnings filtering context managers so that the warnings don't propagate up to the user.

I'm seeing this from afar so take it with a grain of salt, but this approach sounds problematic to me; generally speaking, you should be able to replace internal calls to deprecated APIs with non-deprecated ones. Failing to do so would be a sign that there are no viable substitutes for deprecated APIs, which in turn begs the question of why it's being deprecated at all. Put another way, this approach puts you at risk of painting yourself into a corner, where you may end up having to choose between reverting the deprecation of extending it well beyond its initial scope to include any other public API that relied on the initial target.

@Littie28 your proposal seems great to me. "Ugly" details are allowed to survive the deprecation if they don't reach the public interface, where the margin of error is much narrower.

[jagerber48]

@neutrinoceros

does AMICAL call error_components() directly?

thanks for asking. Indeed I failed to recognise that AMICAL didn't even call this method directly. What it's actually calling is AffineScalarFunc.std_dev, which in turns calls its error_component method internally.

Great, thanks for the info. So you wouldn't have been affected by the intended behavior. Only the bug of internal usages propagating to the user gave you the issue. This another data point towards "this feature is rarely used so it may be ok to deprecate it without a migration path". Though I do still acknowledge that doing this is not a best practice. Again, it is an effort to clean up the API where we decided error_components is better semantically than error_components().

A longer migration path would be to

• introduce calc_error_components() method at the same time as deprecating error_components() method.
• Then eliminate error_components() method in favor of error_components property while users have migrated to calc_error_components()
• Then eliminate calc_error_components().

This approach would never force users to change their code at the same time as downloading a new version of uncertainties but it introduces the obnoxious, and temporary, calc_error_components() method so users would actually have to change code twice. This does not seem like a better option.

In my opinion we should have guarded internal usages with warnings filtering context managers so that the warnings don't propagate up to the user.

I'm seeing this from afar so take it with a grain of salt, but this approach sounds problematic to me; generally speaking, you should be able to replace internal calls to deprecated APIs with non-deprecated ones. Failing to do so would be a sign that there are no viable substitutes for deprecated APIs, which in turn begs the question of why it's being deprecated at all. Put another way, this approach puts you at risk of painting yourself into a corner, where you may end up having to choose between reverting the deprecation of extending it well beyond its initial scope to include any other public API that relied on the initial target.

You are right that the deprecation causing problems with internal calls is a code smell flagging exactly the issue you're raising here about the migration path. In 4.0 we use a different strategy for calculating uncertainty propagation that doesn't rely on the error_components property. In fact, we considered eliminating error_components entirely because of the low usage, but there is a certain use case involving tagging sources of uncertainties that compelled us enough to keep it in 4.0.

---

@neutrinoceros for the long term problem of releasing 4.0 (not for resolving the short term bug users are encountering with version 3.2.4) I see 3 approaches:

1. Continue as planned. Mark error_components() method as deprecated in 3.x. Remove it in 4.0 and add error_components property. This approach has the lack of migration strategy you are concerned about and as I acknowledge, but, as I've pointed out, there are not many users of this function so we calculated that it is tolerable.
2. Avoid the deprecation entirely. Leave error_components() as an instance method into 4.0
3. Continue to mark error_components() method as deprecated in 3.x but also introduce a fodder method called calc_error_components() in 3.x. In 4.x we could remove error_components(), mark calc_error_components() as deprecated and add error_components instance property. Finally in 5.x we could remove calc_error_components() method. This approach solves the "no way for users to upgrade code cleanly" problem, but users who actually use error_components() would end up changing code twice. This is a worse case in my opinion.

Feedback on these three approaches would be very welcome, especially from someone who uses error_components() explicitly now.