-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Deprecate inplace #2524
Deprecate inplace #2524
Changes from 6 commits
10fc573
edf1a0a
3a5cf5d
b1ed4df
484d71f
63d4175
66d3cea
4359403
62faebc
7a0d37b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
requires_scipy, source_ndarray) | ||
|
||
|
||
@pytest.mark.filterwarnings('ignore:The inplace argument') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we try fixing these tests now instead? Otherwise we may be stuck with large clean-up later. For testing deprecations, I find it often helps to convert warnings into an error, e.g., by adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we might want to keep those tests around while people are still using that code path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot the main reason for doing this: to make sure that xarray doesn't rely upon the deprecated behavior internally. But if you already checked that, then I suppose this is fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I haven't checked that closely but I can do that tonight. |
||
class TestDataArray(object): | ||
@pytest.fixture(autouse=True) | ||
def setup(self): | ||
|
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.
Can you add the
stacklevel
argument with an appropriate value? (you'll need to experiment a bit)That ensures that warnings end up pointing to the specific line of code that need to be updated.
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.
stacklevel=3
seems to give useful warnings with the test suite