Change the scope of warning settings to avoid import time side effects

[freeman-lab]

Hi team! Super cool project here. We're excited to be using it, and so far it's working great.

One thing we noticed is that the warnings filter set in vector.py and spline.py and elsewhere, where you call

warnings.simplefilter("default")

is specified such that simply by calling import verde anywhere else overrides existing warning filters with this particular, extremely broad setting.

As a simple example, in a fresh Python 3.7.6 shell I can call

import pickle
pickle.dump('test', open('test.pkl', 'wb'))

and I see no warning, but if I call

import verde
import pickle
pickle.dump('test', open('test.pkl', 'wb'))

I see

/usr/local/bin/ipython:1: ResourceWarning: unclosed file <_io.BufferedWriter name='test.pkl'>
  #!/usr/local/opt/python/bin/python3.7
ResourceWarning: Enable tracemalloc to get the object allocation traceback

In practice, this means we need to manually change a bunch of warning settings in a downstream package simply because we are importing verde!

Any chance you might be open to placing your warning settings inside a context manager? Something like this:

import warnings

with warnings.catch_warnings():
    warnings.simplefilter("default")
    <any code that triggers the warnings you want to hide>

That would prevent the settings in verde from affecting other packages. If you're interested in going this route, happy to help clarify how to do it.

To see this behavior I'm running Python 3.7.6 via homebrew, OS 10.5.5, and verde 1.5.0.

[welcome]

👋 Thanks for opening your first issue here! Please make sure you filled out the template with as much detail as possible.

You might also want to take a look at our Contributing Guide and Code of Conduct.

[santisoler]

Hi @freeman-lab ! Thanks a lot for raising this issue.
I agree with you, it's not a good practice for a single library to override such global configuration on import that might affect the behaviour of any other library.

After a quick view I saw that we are only changing the warnings simplefilter to show DeprecationWarnings, so we should make the modifications you propose on those files.

Would you like to open a PR to solve this?

[leouieda]

Sorry, that was my fault 🙁 I did that because deprecation warnings weren't being shown by default (which is strange to me since it makes them kind of useless).

@freeman-lab to clarify, you're suggesting setting the filter temporarily whenever we issue a warning instead of doing it at the module level? I agree with @santisoler that this something we should do and would be happy if this is something you'd like to take on.