Skip to content

MAINT: Added stacklevel argument to warnings.warn #6424

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

Closed

Conversation

sumitbinnani
Copy link

Updated numpy/lib/function_base.py.
Fixes #5945

@sumitbinnani sumitbinnani changed the title MAINT: Added stacklevel argument to warnings.warn. Fixes #5945 MAINT: Added stacklevel argument to warnings.warn Oct 7, 2015
@@ -1004,7 +1004,7 @@ def select(condlist, choicelist, default=0):
# 2014-02-24, 1.9
warnings.warn("select with an empty condition list is not possible"
"and will be deprecated",
DeprecationWarning)
DeprecationWarning, stacklevel = 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No spaces around the = in keyword assignments.

@jaimefrio
Copy link
Member

Some style nitpicks...

@charris
Copy link
Member

charris commented Oct 8, 2015

Umm, we don't allow tabs, use spaces please.

@sumitbinnani sumitbinnani force-pushed the AddingStackLevelWarning branch from a1cf3a4 to de9a045 Compare October 8, 2015 16:11
Fixed indentation to comply with PEP8
Used ```spacecs``` in place of ```tabs```
@sumitbinnani sumitbinnani force-pushed the AddingStackLevelWarning branch from de9a045 to 84ae950 Compare October 8, 2015 16:13
@sumitbinnani
Copy link
Author

@charris @jaimefrio
Have fixed indentation issues and used spaces as advised. Kindly review the code and let me know if any more changes are required.

@sumitbinnani
Copy link
Author

Hi @charris @jaimefrio

Does the changes meet the requirements? Kindly let me know if any additional changes are required.

@sumitbinnani
Copy link
Author

Hi @charris and @jaimefrio,

Is there any issue with the patch? Kindly let me know and would try to submit the patch for the same asap.

@njsmith
Copy link
Member

njsmith commented Nov 12, 2015

The changes in the PR look good to me now.

Question 1: the PR summary says that this fixes #5945, but the main question in #5945 is about why a call to delete was issuing a warning that referred to insert -- does this PR actually address that? (Should it?)

Question 2: any chance you could add some tests for this? I guess adding tests for all the little deprecation warning tweaks would be tiresome and of limited value, because the code will just get removed again in a few versions anyway. But for the RuntimeWarnings in cov, _median, and _percentile it seems particularly useful, because the RuntimeWarnings will be around indefinitely, and because it's not 100% obvious whether stacklevel=2 is even correct for _median and _percentile which are internal helper functions. (It might be because I think the code the callers are written in C and thus don't affect the stacklevel= count, but it's not 100% obvious.) The way to do this would be to use the catch_warnings context manager with record=True, call the function that should issue a warning, and then check that the .filename attribute of the recorded warning object refers to the file that called the function rather than some internal numpy file.

Sorry for the slow response, and thanks for being persistent in pinging us!

@sumitbinnani
Copy link
Author

Thanks @njsmith for the inputs. I will look into the details during weekend and update the thread.

@homu
Copy link
Contributor

homu commented Feb 7, 2016

☔ The latest upstream changes (presumably #7181) made this pull request unmergeable. Please resolve the merge conflicts.

@sumitbinnani
Copy link
Author

Can you elaborate on what needs to be done, @homu?

@rkern
Copy link
Member

rkern commented Feb 15, 2016

@homu is a bot and will not reply. You will need to manually merge in master and fix the conflicts.

@charris
Copy link
Member

charris commented Sep 2, 2016

This has been taken care of in #7148 along with a number of other cases, so closing this. Thanks @sumitbinnani, I apologize that we didn't get this in earlier.

@charris charris closed this Sep 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants