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
On Wed, Jan 27, 2016 at 7:26 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote:
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?
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.
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
On 2016-01-27 21:01, Ralf Gommers wrote:
On Wed, Jan 27, 2016 at 7:26 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote:
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?
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.
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 NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
On Wed, Jan 27, 2016 at 11:02 PM, sebastian <sebastian@sipsolutions.net> wrote:
On 2016-01-27 21:01, Ralf Gommers wrote:
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.p... Probably more trouble than it's worth though. Ralf
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 On Wed, Jan 27, 2016 at 5:07 PM, Ralf Gommers <ralf.gommers@gmail.com> wrote:
On Wed, Jan 27, 2016 at 11:02 PM, sebastian <sebastian@sipsolutions.net> wrote:
On 2016-01-27 21:01, Ralf Gommers wrote:
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.p...
Probably more trouble than it's worth though.
Ralf
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
On Mi, 2016-01-27 at 17:12 -0500, Benjamin Root wrote:
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
Ben Root
On Wed, Jan 27, 2016 at 5:07 PM, Ralf Gommers <ralf.gommers@gmail.com
wrote:
On Wed, Jan 27, 2016 at 11:02 PM, sebastian < sebastian@sipsolutions.net> wrote:
On 2016-01-27 21:01, Ralf Gommers wrote:
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 NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
+1 On Wed, Jan 27, 2016 at 10:26 AM, Sebastian Berg <sebastian@sipsolutions.net
wrote:
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 _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
-- Nathaniel J. Smith -- https://vorpus.org <http://vorpus.org>
participants (5)
-
Benjamin Root -
Nathaniel Smith -
Ralf Gommers -
sebastian -
Sebastian Berg