Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warnings.filterwarnings changes the behavior of np.asarray #14735

Closed
keewis opened this issue Oct 17, 2019 · 9 comments · Fixed by #14745
Closed

warnings.filterwarnings changes the behavior of np.asarray #14735

keewis opened this issue Oct 17, 2019 · 9 comments · Fixed by #14745

Comments

@keewis
Copy link
Contributor

keewis commented Oct 17, 2019

Using warnings.filterwarnings with a numpy wrapper class changes the behavior of np.asarray (or rather np.array).

I believe the reason for this is the equivalent of an except Exception block (in the C code), since the warning base class Warning inherits from Exception.

Having __len__ and __getitem__ is not required to reproduce, but they make the difference a bit more obvious.

xref hgrecco/pint#892

Reproducing code example:

import warnings
import numpy as np

class NumpyWrapper:
    def __init__(self, array):
        self.array = array

    def __len__(self):
        return len(self.array)

    def __getitem__(self, *args, **kwargs):
        return type(self)(self.array.__getitem__(*args, **kwargs))

    def __getattr__(self, name):
        if name.startswith("__array_"):
            warnings.warn("object got converted", UserWarning)

        return getattr(self.array, name)

    def __repr__(self):
        return f"<Wrapper({self.array})>"

array = NumpyWrapper(np.arange(10))
print(repr(np.asarray(array))  # warning
# [0 1 2 3 4 5 6 7 8 9]
warnings.filterwarnings("error")
print(repr(np.asarray(array))  # no warning, but also no error
# [<Wrapper(0)> <Wrapper(1)> <Wrapper(2)> <Wrapper(3)> <Wrapper(4)>
#  <Wrapper(5)> <Wrapper(6)> <Wrapper(7)> <Wrapper(8)> <Wrapper(9)>]

Numpy/Python version information:

numpy: 1.17.2
python: 3.7.3

@seberg
Copy link
Member

seberg commented Oct 17, 2019

I think this is expected since the getattr fails, and not the __array__ call itself. I suppose we could be specific about it and only catch AttributeError.
But my feeling is that you should change your code to not rely on behaviour within __getattr__.

@keewis
Copy link
Contributor Author

keewis commented Oct 17, 2019

I guess it is, but still, having a warning implicitly silenced (even if it happens to have been raised like an exception) was a surprise for me. The code above is a reduced version of pint, which warns when the units get implicitly stripped by certain methods / properties. This came up within unit tests where I have been trying to test for these warnings.

So I think catching only AttributeError would be ideal (which would allow the user to easier debug their own code), but you could also try to make an exception for Warning (which probably should always bubble up):

try:
    ...
except Warning:
    raise
except Exception:
    ...

@seberg
Copy link
Member

seberg commented Oct 17, 2019

We can discuss that, but what I meant is that I do not understand why the code does not use:

class NumpyWrapper:
    def __array__(self):
        warnings.warn(...)

which I believe works as expected.

@keewis
Copy link
Contributor Author

keewis commented Oct 17, 2019

I think that's because pint also works without numpy, so there is more going on in __getattr__ than just the warning. But apart from that, I don't really know.

@jthielen, do you have an opinion on this?

@jthielen
Copy link

@keewis I've been uncomfortable with how pint uses a __getattr__ fallback to handle __array_* attributes/methods, but this issue appears to be a good demonstration of how it is problematic. I would think that this, along with hgrecco/pint#878 (comment) and perhaps also Unidata/MetPy#1209 (comment), should be enough justification for pint to switch over to explicitly defining its __array_* attributes/methods.

So, assuming that explicitly defining __array__ in pint resolves this warning filter issue, this would be a pint, not a numpy issue?

@keewis
Copy link
Contributor Author

keewis commented Oct 17, 2019

both, I guess.

Considering that we can avoid this issue in pint, @seberg, @eric-wieser, how do you want to handle this?

@seberg
Copy link
Member

seberg commented Oct 17, 2019

I am good with making it except AttributeError, that should be OK; I personally do not care much for it though. It does not seem awful to me as is either.

@keewis
Copy link
Contributor Author

keewis commented Oct 17, 2019

then, because it seems to be the correct way to me: do you think you could change it to except AttributeError? And we will try to rewrite that part of pint, I think.

@seberg
Copy link
Member

seberg commented Oct 17, 2019

Sure PRs welcome ;). But I still think you should try and rewrite it. Note that it would be sufficient to return a lambda which gives the warning and returns the result...

mattip pushed a commit that referenced this issue Oct 30, 2019
…#14745)

* TST: check whether filtered warnings change behaviour in array()

Setting warnings from a converted object's `__getattr__` to errors
changes the behaviour of `numpy.array`. See #14735.

* MAINT: clear errors raised by `PyArray_LookupSpecial*`
mproszewska pushed a commit to mproszewska/numpy that referenced this issue Nov 18, 2019
…numpy#14745)

* TST: check whether filtered warnings change behaviour in array()

Setting warnings from a converted object's `__getattr__` to errors
changes the behaviour of `numpy.array`. See numpy#14735.

* MAINT: clear errors raised by `PyArray_LookupSpecial*`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants