Should not call `warnings.filterwarnings` in a library

[mehaase]

Description

Importing sklearn changes the current warning filters, making it very difficult to debug warnings using standard Python syntax, e.g -W command flag.

Steps/Code to Reproduce

The module sklearn.cross_validation is deprecated, and importing it results in a warning.

$ python3.6 -c 'import mypackage'
/usr/local/lib/python3.6/dist-packages/sklearn/cross_validation.py:41: DeprecationWarning:
This module was deprecated in version 0.18 in favor of the model_selection module into
which all the refactored classes and functions are moved. Also note that the interface of
the new CV iterators are different from that of this module. This module will be removed
in 0.20.
  "This module will be removed in 0.20.", DeprecationWarning)

Okay, I'm surprised to see a DeprecationWarning — that's not supposed to happen in Python unless I ask to see it — but let me try to get a stack trace so I can see where it's coming from.

$ python3.6 -W error::DeprecationWarning:: -c 'import mypackage'
/usr/local/lib/python3.6/dist-packages/sklearn/cross_validation.py:41: DeprecationWarning:
This module was deprecated in version 0.18 in favor of the model_selection module into
which all the refactored classes and functions are moved. Also note that the interface of
the new CV iterators are different from that of this module. This module will be removed
in 0.20.
  "This module will be removed in 0.20.", DeprecationWarning)

Oh that's weird, the -W error::DeprecationWarning:: should raise the warning an exception, which would show me a full stack trace and tell me where sklearn.cross_validation is being imported from. Why does the configuration flag have no effect?

I tried playing with the flags for a while, then eventually realized that it is not possible to change sklearn's warning filter, because it is hardcoded in the module!

Expected Results

1. At the very least, honor the user's warning filters and don't change the warning filter to something that the user didn't ask for.
2. Ideally, don't configure warning filters at all. This behavior should be controlled by an application or an end user, not a library.

If the warnings.filterwarnings(…) is removed completely, then the result would be expected Python behavior and I would be able to debug this very quickly:

$ python3.6 -W error::DeprecationWarning:: -c 'import mypackage'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/local/lib/python3.6/dist-packages/mypackage/__init__.py", line 5, in <module>
    from .classifiers import (
  File "/usr/local/lib/python3.6/dist-packages/formasaurus/classifiers.py", line 8, in <module>
    from mypackage import formtype_model, mypackage
  File "/usr/local/lib/python3.6/dist-packages/mypackage/formtype_model.py", line 9, in <module>
    from mypackage.annotation import get_annotation_folds
  File "/usr/local/lib/python3.6/dist-packages/mypackage/annotation.py", line 5, in <module>
    from sklearn.cross_validation import LabelKFold
  File "/usr/local/lib/python3.6/dist-packages/sklearn/cross_validation.py", line 41, in <module>
    "This module will be removed in 0.20.", DeprecationWarning)
DeprecationWarning: This module was deprecated in version 0.18 in favor of the model_selection module into which all the refactored classes and functions are moved. Also note that the interface of the new CV iterators are different from that of this module. This module will be removed in 0.20.

sklearn is not special enough to break with standard Python conventions.

[mehaase]

This issue leads to other confusion, e.g. #7963

[jnothman]

You're right that we may be being bad citizens. We've forcibly shown the DeprecationWarnings for a long time now.

And I can't see an easy way to determine whether the user has tried to override default settings. The presence of DeprecationWarning twice in warnings.filters?

One way to change this would be to make a custom deprecation warning class that does not inherit from DeprecationWarning. There are questions of backwards compatibility there...

[lesteve]

@mehaase I think that is just an issue of principles vs real world and I am afraid that the latter tends to win in the long run. If there is a way to be a better citizen, given the constraints mentioned by @jnothman (look at #7963 and probably the associated PR #7995 for more details), then a PR is more than welcome.

If not, for the few cases where this is annoying, I would just recommend to comment out the line in sklearn/__init__.py and carry on with your life.

For the record, numpy modifies warnings.filters as does ipython.

[lesteve]

And while I am at it, let me mention this: we may be able to use the stacklevel argument in warnings.warn a bit better, if we were using it correctly (and with tests), the warnings would show you the offending line and you would not need to turn warnings into errors.

In @mehaase case, the warnings would point at where he imports sklearn.cross_validation, rather than sklearn/cross_validation.py which he can not do anything about. Maybe that is what #7995 was partially about, I don't remember all the details.

[jnothman]

It sounds like it's an issue with python, if libraries are able to (and are forced to to set defaults) override some user discussed filters. Is there a good way for library developers to do this?

[basnijholt]

In the Adaptive documentation we use Juypter-sphinx to execute code.

Whenever there is a warning, Jupyter-sphinx will raise an error. This result in failed builds.

Unfortunately, because of this issue here, filtering out the warnings (like done here) doesn't work.

This issue is really quite annoying and means that we have to pin the old scikit-learn in order to suppress the warning.

Like many others have remarked before (i.e. #11792), this is very bad behavior.

[amueller]

@basnijholt suggestions for solutions are welcome.
The behavior we want is that our warnings are shown by default. However, people that have explicitly filtered our warnings previously should probably also not receive our warnings.
If we replace DeprecationWarning with a custom class, people that ignored our warnings with any warning filter using DeprecationWarning will get the warning again.

As a workaround for your case, can't you import sklearn and then set the filter?

Maybe we should just make the backward-incompatible change to a custom warning. I'm quite sure it'll annoy way more users than the current situation does, though.

[amueller]

We should probably also figure out why "append" doesn't work:
#11792 (comment)

[thomasjpfan]

We should probably also figure out why "append" doesn't work:

The default warning filters are:

default::DeprecationWarning:__main__
ignore::DeprecationWarning
ignore::PendingDeprecationWarning
ignore::ImportWarning
ignore::ResourceWarning

which will filter out DeprecationWarnings not in __main__. If we use append, the sklearn filter warning is on the bottom (appended). This means, the ignore::DeprecationWarning will filter out the warning before it gets to our always::sklearn. filter.

[amueller]

We could check if the user wanted to error and then leave it at that to fix a small part of the issue.
I don't see how we could fix the real issue without breaking backward compatibility but maybe we should do it as a bug fix?

[NicolasHug]

If I'm using cool_package that in turn is using scikit-learn in do_some_stuff(), then I have zero way to filter out the deprecation warnings from sklearn when I use cool_package.do_some_stuff().

The only way to avoid the deprecation warnings from sklearn is to filter them out after sklearn is imported, and only cool_package can do that.

---

As previously mentioned we can make the issue go away by appending an 'always' filter and use a NewDepWarning class (that doesn't inherit from DeprecationWarning).

class NewDepWarning(Warning):
    pass
warnings.simplefilter('always', NewDepWarning, append=True)

This way users can place warnings.simplefilter('ignore', sklearn.NewDepWarning) wherever they want.

This is backward incompatible in the sense that users who already had their filterwarning('ignore', DeprecationWarning) working will now still see the NewDepWarnings.

So we will make them unhappy for a little while. But I think it's a necessary evil that's worth it for the long term. People are already unhappy right now anyway.

[amueller]

If we add a new class, we don't need to add a filter, right? Unless we inherit from DeprecationWarning? That might make it unnecessarily tricky though.

[NicolasHug]

Oh you're right we don't need the filter.

I'm not sure we can do what we want and inherit from DeprecationWarning though.