[Numpy-discussion] Bump warning stacklevel

[Sebastian Berg]

Hi all,

in my PR about warnings suppression, I currently also have a commit
which bumps the warning stacklevel to two (or three), i.e. use:

warnings.warn(..., stacklevel=2)

(almost) everywhere. This means that for example (take only the empty
warning):

np.mean([])

would not print:

/usr/lib/python2.7/dist-packages/numpy/core/_methods.py:55:
RuntimeWarning: Mean of empty slice.
warnings.warn("Mean of empty slice.", RuntimeWarning)

but instead print the actual `np.mean([])` code line (the repetition of
the warning command is always a bit funny).

The advantage is nicer printing for the user.

The disadvantage would probably mostly be that existing warning filters
that use the `module` keyword argument, will fail.

Any objections/thoughts about doing this change to try to better report
the offending code line? Frankly, I am not sure whether there might be
a python standard about this, but I would expect that for a library
such as numpy, it makes sense to change. But, if downstream uses
warning filters with modules, we might want to reconsider for example.

- Sebastian

[Ralf Gommers]

Post by Sebastian Berg
Hi all,
in my PR about warnings suppression, I currently also have a commit
warnings.warn(..., stacklevel=2)
(almost) everywhere. This means that for example (take only the empty
np.mean([])
RuntimeWarning: Mean of empty slice.
warnings.warn("Mean of empty slice.", RuntimeWarning)
but instead print the actual `np.mean([])` code line (the repetition of
the warning command is always a bit funny).
The advantage is nicer printing for the user.
The disadvantage would probably mostly be that existing warning filters
that use the `module` keyword argument, will fail.
Any objections/thoughts about doing this change to try to better report
the offending code line?

This has annoyed me for a long time, it's hard now to figure out where
warnings really come from. Especially when running something large like
scipy.test(). So +1.

Post by Sebastian Berg
Frankly, I am not sure whether there might be
a python standard about this, but I would expect that for a library
such as numpy, it makes sense to change. But, if downstream uses
warning filters with modules, we might want to reconsider for example.

There probably are usages of `module`, but I'd expect that it's used a lot
less than `category` or `message`. A quick search through the scipy repo
gave me only a single case where `module` was used, and that's in
deprecated weave code so soon the count is zero. Also, even for relevant
usage, nothing will break in a bad way - some more noise or a spurious test
failure in numpy-using code isn't the end of the world I'd say.

One issue will be how to keep this consistent. `stacklevel` is used so
rarely that new PRs will always omit it for new warnings. Will we just rely
on code review, or would a private wrapper around `warn` to use inside
numpy plus a test that checks that the wrapper is used everywhere be
helpful here?

Ralf

[sebastian]

On Wed, Jan 27, 2016 at 7:26 PM, Sebastian Berg

Post by Sebastian Berg
Hi all,
in my PR about warnings suppression, I currently also have a commit
warnings.warn(..., stacklevel=2)
(almost) everywhere. This means that for example (take only the empty
np.mean([])
RuntimeWarning: Mean of empty slice.
warnings.warn("Mean of empty slice.", RuntimeWarning)
but instead print the actual `np.mean([])` code line (the repetition of
the warning command is always a bit funny).
The advantage is nicer printing for the user.
The disadvantage would probably mostly be that existing warning filters
that use the `module` keyword argument, will fail.
Any objections/thoughts about doing this change to try to better report
the offending code line?

This has annoyed me for a long time, it's hard now to figure out where
warnings really come from. Especially when running something large
like scipy.test(). So +1.

Post by Sebastian Berg
Frankly, I am not sure whether there might be
a python standard about this, but I would expect that for a library
such as numpy, it makes sense to change. But, if downstream uses
warning filters with modules, we might want to reconsider for
example.

There probably are usages of `module`, but I'd expect that it's used a
lot less than `category` or `message`. A quick search through the
scipy repo gave me only a single case where `module` was used, and
that's in deprecated weave code so soon the count is zero. Also, even
for relevant usage, nothing will break in a bad way - some more noise
or a spurious test failure in numpy-using code isn't the end of the
world I'd say.
One issue will be how to keep this consistent. `stacklevel` is used so
rarely that new PRs will always omit it for new warnings. Will we just
rely on code review, or would a private wrapper around `warn` to use
inside numpy plus a test that checks that the wrapper is used
everywhere be helpful here?

Yeah, I mean you could add tests for the individual functions in
principle.
I am not sure if adding an alias helps much, how are we going to test
that
warnings.warn is not being used? Seems like quite a bit of voodoo
necessary
for that.

- Sebastian

Ralf
_______________________________________________
NumPy-Discussion mailing list
https://mail.scipy.org/mailman/listinfo/numpy-discussion

[Ralf Gommers]

Post by Ralf Gommers
One issue will be how to keep this consistent. `stacklevel` is used so
rarely that new PRs will always omit it for new warnings. Will we just
rely on code review, or would a private wrapper around `warn` to use
inside numpy plus a test that checks that the wrapper is used
everywhere be helpful here?

Yeah, I mean you could add tests for the individual functions in principle.
I am not sure if adding an alias helps much, how are we going to test that
warnings.warn is not being used? Seems like quite a bit of voodoo necessary
for that.

I was thinking something along these lines, but with a regexp checking for
warnings.warn:
https://github.com/scipy/scipy/blob/master/scipy/fftpack/tests/test_import.py

Probably more trouble than it's worth though.

Ralf

[Benjamin Root]

I like the idea of bumping the stacklevel in principle, but I am not sure
it is all that practical. For example, if a warning came up when doing "x /
y", I am assuming that it is emitted from within the ufunc np.divide(). So,
you would need two stacklevels based on whether the entry point was the
operator or a direct call to np.divide()? Also, I would imagine it might
get weird for numpy functions called within other numpy functions. Or
perhaps I am not totally understanding how this would be done?

Ben Root

Post by Ralf Gommers

Post by Ralf Gommers
One issue will be how to keep this consistent. `stacklevel` is used so
rarely that new PRs will always omit it for new warnings. Will we just
rely on code review, or would a private wrapper around `warn` to use
inside numpy plus a test that checks that the wrapper is used
everywhere be helpful here?

Yeah, I mean you could add tests for the individual functions in principle.
I am not sure if adding an alias helps much, how are we going to test that
warnings.warn is not being used? Seems like quite a bit of voodoo necessary
for that.

I was thinking something along these lines, but with a regexp checking for
https://github.com/scipy/scipy/blob/master/scipy/fftpack/tests/test_import.py
Probably more trouble than it's worth though.
Ralf
_______________________________________________
NumPy-Discussion mailing list
https://mail.scipy.org/mailman/listinfo/numpy-discussion

[Sebastian Berg]

Post by Benjamin Root
I like the idea of bumping the stacklevel in principle, but I am not
sure it is all that practical. For example, if a warning came up when
doing "x / y", I am assuming that it is emitted from within the ufunc
np.divide(). So, you would need two stacklevels based on whether the
entry point was the operator or a direct call to np.divide()? Also, I
would imagine it might get weird for numpy functions called within
other numpy functions. Or perhaps I am not totally understanding how
this would be done?

You are right of course, and the answer is that I was never planning on
fixing that. This is only for warnings by functions directly called by
the users (or those which always go through one more level, though I
did not check the C-api for such funcs). Also C-Api warnings are
already correct in this regard automatically.

Anyway, I still think it is worth it, even if in practice a lot of
warnings are such things as ufunc warnings from inside a python func.
And there is no real way to change that, that I am aware of, unless
maybe we add a warning_stacklevel argument to those C-api funcs ;).

- Sebastian

Post by Benjamin Root
Ben Root

On Wed, Jan 27, 2016 at 11:02 PM, sebastian <

Post by Ralf Gommers
One issue will be how to keep this consistent. `stacklevel` is used so
rarely that new PRs will always omit it for new warnings. Will we just
rely on code review, or would a private wrapper around `warn` to use
inside numpy plus a test that checks that the wrapper is used
everywhere be helpful here?

Yeah, I mean you could add tests for the individual functions in principle.
I am not sure if adding an alias helps much, how are we going to test that
warnings.warn is not being used? Seems like quite a bit of voodoo necessary
for that.

I was thinking something along these lines, but with a regexp
checking for warnings.warn: https://github.com/scipy/scipy/blob/mas
ter/scipy/fftpack/tests/test_import.py
Probably more trouble than it's worth though.
Ralf
_______________________________________________
NumPy-Discussion mailing list
https://mail.scipy.org/mailman/listinfo/numpy-discussion

_______________________________________________
NumPy-Discussion mailing list
https://mail.scipy.org/mailman/listinfo/numpy-discussion

[Nathaniel Smith]

+1

Post by Sebastian Berg
Hi all,
in my PR about warnings suppression, I currently also have a commit
warnings.warn(..., stacklevel=2)
(almost) everywhere. This means that for example (take only the empty
np.mean([])
RuntimeWarning: Mean of empty slice.
warnings.warn("Mean of empty slice.", RuntimeWarning)
but instead print the actual `np.mean([])` code line (the repetition of
the warning command is always a bit funny).
The advantage is nicer printing for the user.
The disadvantage would probably mostly be that existing warning filters
that use the `module` keyword argument, will fail.
Any objections/thoughts about doing this change to try to better report
the offending code line? Frankly, I am not sure whether there might be
a python standard about this, but I would expect that for a library
such as numpy, it makes sense to change. But, if downstream uses
warning filters with modules, we might want to reconsider for example.
- Sebastian
_______________________________________________
NumPy-Discussion mailing list
https://mail.scipy.org/mailman/listinfo/numpy-discussion