Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Automagically set the stacklevel on warnings.#11298
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This looks like a good idea to me, particularly for pyplot.... |
Great idea! 😄 While we're at it: Can we make that behavior configurable? While a user is not interested in the internals of matplotlib, it is really helpful for debugging purposes to see where a warning was really generated. |
anntzer commentedMay 24, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
What kind of config do you propose? Note that you can already do (as of this PR) to achieve that :-) Edit: Also, you can always set the warnings filter to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems fine, but I didn't pull and test...
@anntzer Stacklevel override: That's a bit more safe than messing with |
40369b5 to371f801CompareI got rid of the stacklevel argument as it literally goes against the point of the function, and renamed it to _warn_external, which is perhaps a bit more explicit (if you want to use stacklevel, just use warnings.warn, again there's no point of keeping around half-incorrect stacklevels at each call site). Honestly, given that the thing is private, I think if one wants to know where the warning actually comes from, one should just do |
+1 for A final request: Can you add a note to the docstring: "If you need the full stacktrace for debugging purposes, you can do Otherwise it may not be obvious how to get the additional info. Also it states the intent that the function signature should stay compatible with |
done |
There are many places in Matplotlib where it is impossible to set astatic stacklevel on warnings that works in all cases, because a samefunction may either be called directly or via some wrapper (e.g.pyplot).Instead, compute the stacklevel by walking the stack. Given thatwarnings refer to conditions that should, well, be avoided, I believewe don't need to worry too much about the runtime cost.As an example, use this mechanism for the "ambiguous second argument toplot" warning. Now both```plt.gca().plot("x", "y", data={"x": 1, "y": 2})```and```plt.plot("x", "y", data={"x": 1, "y": 2})```emit a warning that refers to the relevant line in the user source,whereas previously the second snippet would refer to the pyplot wrapper,which is irrelevant to the user.Note that this only works from source, not from IPython, as the latteruses `exec` and AFAICT there is no value of stacklevel that correctlyrefers to the string being exec'd.Of course, the idea would be to ultimately use this throughout thecodebase.
There are many places in Matplotlib where it is impossible to set a
static stacklevel on warnings that works in all cases, because a same
function may either be called directly or via some wrapper (e.g.
pyplot).
Instead, compute the stacklevel by walking the stack. Given that
warnings refer to conditions that should, well, be avoided, I believe
we don't need to worry too much about the runtime cost.
As an example, use this mechanism for the "ambiguous second argument to
plot" warning. Now both
and
emit a warning that refers to the relevant line in the user source,
whereas previously the second snippet would refer to the pyplot wrapper,
which is irrelevant to the user.
Note that this only works from source, not from IPython, as the latter
uses
execand AFAICT there is no value of stacklevel that correctlyrefers to the string being exec'd.
Of course, the idea would be to ultimately use this throughout the
codebase.
PR Summary
PR Checklist