[python-package] make LGBMDeprecationWarning inherit from FutureWarning

[jameslamb]

Contributes to #6435 .

LGBMDeprecationWarning was added 7 years ago, in #1046. That was done because Python's builtin DeprecationWarning is not shown by default.

PEP 565, accepted in November 2017, offers a different prescription for this.

For library and framework authors that want to ensure their API compatibility warnings are more reliably seen by their users, the recommendation is to use a custom warning class that derives from DeprecationWarning in Python 3.7+, and from FutureWarning in earlier versions.

(PEP-565)

This change will also ensure such warnings show up in tests of anything using lightgbm which use pytest to run their tests.

From the pytest docs:

By default pytest will display DeprecationWarning and PendingDeprecationWarning warnings from user code and third-party libraries, as recommended by PEP 565. This helps users keep their code modern and avoid breakages when deprecated warnings are effectively removed.

("DeprecationWarning and PendingDeprecationWarning")

[jmoralez]

I think we should use FutureWarning instead, since as the PEP's title suggests, those warnings are only shown when they come from code in the __main__ module

This means that in cases where the nominal location of the warning (as determined by the stacklevel parameter to warnings.warn) is in the main module, the first occurrence of each DeprecationWarning will once again be reported.

So if someone has code wrapping LightGBM in a separate file and call it from another script they'll never see these warnings. With the current setup those warnings are always shown, since UserWarnings are always shown with the default filters:

default::DeprecationWarning:main
ignore::DeprecationWarning
ignore::PendingDeprecationWarning
ignore::ImportWarning
ignore::BytesWarning
ignore::ResourceWarning

That PEP's description of FutureWarning:

FutureWarning: reported by default for all code. The intended audience is users of applications written in Python, rather than other Python developers (e.g. warning about use of a deprecated setting in a configuration file format).

[jmoralez]

Example of pandas using FutureWarning for a deprecated option:

https://github.com/pandas-dev/pandas/blob/d9cdd2ee5a58015ef6f4d15c7226110c9aab8140/pandas/__init__.py#L196-L204

[jameslamb]

Gah I'm just not sure. I had FutureWarning originally when I started working on this, then read that line in the PEP that said seemed to pretty explicitly recommend using a custom extension of DeprecationWarning if you are a library/framework (which LightGBM is).

examples of pandas using FutureWarning

I also see many examples of pandas using DeprecationWarning (directly, not a custom extension) to warn about deprecations. https://github.com/search?q=repo%3Apandas-dev%2Fpandas%20DeprecationWarning&type=code

For example: https://github.com/pandas-dev/pandas/blob/d7655475b612baf24d5c4fee1dfe3c971d622824/pandas/core/dtypes/common.py#L214-L219

Interestingly, looks like numpy uses a custom deprecation warning inherited from UserWarning.

class ModuleDeprecationWarning(DeprecationWarning):
    """Module deprecation warning.

    .. warning::

        This warning should not be used, since nose testing is not relevant
        anymore.

    The nose tester turns ordinary Deprecation warnings into test failures.
    That makes it hard to deprecate whole modules, because they get
    imported by default. So this is a special Deprecation warning that the
    nose tester will let pass without making tests fail.

    """
    pass


class VisibleDeprecationWarning(UserWarning):
    """Visible deprecation warning.

    By default, python will not show deprecation warnings, so this class
    can be used when a very visible warning is helpful, for example because
    the usage is most likely a user bug.

    """
    pass

https://github.com/numpy/numpy/blob/2e354ee40c5cb8e8a7ef675a151eef75f557f77e/numpy/exceptions.py#L59-L84

But then sometimes doesn't even use it, and uses DeprecationWarning directly.

https://github.com/numpy/numpy/blob/2e354ee40c5cb8e8a7ef675a151eef75f557f77e/numpy/_core/_internal.py#L198-L202

I don't know what to do any more 😭 . Maybe we should just leave this alone (close this PR) until there's a clearer reason to prefer one specific pattern to another?

[borchero]

Reading through PEP-565, I'd be in favor of using DeprecationWarning although I do find the phrasing in the PEP a little conflicting 👀

[jmoralez]

My main concern here is the actual outcome with respect to warning category, rather than the theoretical intent. Consider the following example:

Suppose I have a file deprecated.py with the following

import warnings

def f():
    warnings.warn("hi", category=DeprecationWarning)


if __name__ == '__main__':
    f()

which emulates what LightGBM would be doing.

And another file my_wrapper.py where I use that deprecated code:

from deprecated import f

def g():
    f()

if __name__ == '__main__':
    g()

If I run python deprecated.py I get the expected warning:

/hdd/github/LightGBM/my_module/deprecated.py:4: DeprecationWarning: hi
  warnings.warn("hi", category=DeprecationWarning)

However, if I run python my_wrapper.py I get nothing.

If I change the warning category to FutureWarning or UserWarning and run python my_wrapper.py I get the warning.

So we'd be basically changing the warning visibility and people that wrap LightGBM code wouldn't be aware of the deprecation.

[jameslamb]

I've read back through the PEP, and I agree with @jmoralez 's reasoning... we should use FutureWarning. I think this example is very convincing: #6447 (comment).

I think we want the warnings to be loud, to move as many downstream users (both library developers and people training/serving models directly) to non-deprecated behavior as possible before future releases that remove that behavior.

I've pushed a commit switching it to FutureWarning.

Also note that the only uses of this right now are for warnings that have never been in a lightgbm release (#6446), and hasn't used that warning in (I think) a few years, so this shouldn't break anyone's existing warnings-filtering of lightgbm code.

[borchero]

Makes sense to me!

[jmoralez]

Thanks! I think we should just update the title.

[jameslamb]

🙈 absolutely right, thanks @jmoralez . Just updated it.