Library does not respect warning filters

anuppari commented 4 years ago

Expected Behavior

When warning filters are applied, I'd expect the warnings to be suppressed.

import warnings
from deprecated import deprecated

warnings.simplefilter("ignore")

@deprecated(version='1.2.1', reason="deprecated function")
def fun():
    print("fun")

fun()

Actual Behavior

The script emits a warning message:

test_dep.py:10: DeprecationWarning: Call to deprecated function (or staticmethod) fun. (deprecated function) -- Deprecated since version 1.2.1.
  fun()
fun

Environment

Python version: 3.6.9
Deprecated version: 1.2.7

tantale commented 4 years ago

One possibility is to use the action kwarg:

@deprecated(version='1.2.1', reason="deprecated function", action="ignore")
def fun():
    print("fun")

Does it fix your problem?

anuppari commented 4 years ago

That was a minimum working example. In actuality, we use the deprecated decorator in a library, and want to allow clients to decide which warnings to suppress. Setting the action to "ignore" seems to negate the whole point of having the decorator in the first place.

anuppari commented 4 years ago

The documentation seems to suggest that the python warning control system is used, which should allow users to control warnings. Even the command line warning suppression doesn't work (python -W ignore script.py), let alone the warning control filters.

tantale commented 4 years ago

The current @deprecated behavior is to call warnings.simplefilter(action) (where action is "always" by default, see The Warnings Filter) each time the function is call. That way, this ensure the warning message is shown: when warnings.warn() is called, people expect to see a warning message. And they are happy with that.

The problem with warnings.simplefilter(...) is that it update or reset a global list of filters. filters is a module-level variable defined as follow (Python 3.7 implementation):

filters = [
    (
        'default',
        None,
        DeprecationWarning,
        '__main__',
        0,
    ),
    (
        'ignore',
        None,
        '<value is a self-reference, replaced by this string>',
        None,
        0,
    ),
    (
        'ignore',
        None,
        PendingDeprecationWarning,
        None,
        0,
    ),
    (
        'ignore',
        None,
        ImportWarning,
        None,
        0,
    ),
    (
        'ignore',
        None,
        ResourceWarning,
        None,
        0,
    ),
]

So, the current implementation modify this global variable on every call: this is bad.

A correct implementation of the @deprecated operator could be:

if the action is not provided (None): do not set a filter. That way, when emitting a warning, it takes the current filter (in the filters list). This is what you want: the main function can change the filters according to the user needs.
if the action is provided, we can use the current behavior (which restores the filter at the end of the processing).

I'll see if this implementation is feasible…

tantale / deprecated