-
-
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
Improve open_mfdataset deprecation warnings #3101
Conversation
TomNicholas
commented
Jul 12, 2019
- Closes auto_combine warnings with open_mfdataset #3091
- Tests added
Hello @TomNicholas! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-07-12 13:19:40 UTC |
Codecov Report
@@ Coverage Diff @@
## master #3101 +/- ##
==========================================
- Coverage 95.99% 95.99% -0.01%
==========================================
Files 63 63
Lines 12792 12796 +4
==========================================
+ Hits 12280 12283 +3
- Misses 512 513 +1 |
Awesome. I think this looks great. Sorry for not catching it earlier. |
That's perfect! Thanks @TomNicholas |
@@ -523,7 +523,8 @@ def combine_by_coords(datasets, compat='no_conflicts', data_vars='all', | |||
|
|||
|
|||
def auto_combine(datasets, concat_dim='_not_supplied', compat='no_conflicts', | |||
data_vars='all', coords='different', fill_value=dtypes.NA): | |||
data_vars='all', coords='different', fill_value=dtypes.NA, |
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.
Don't worry about it here -- it's not worth the trouble for a deprecated function -- but in the future when you want something like this the clean way to do it is probably to make a private helper function _auto_combine
that adds the extra non-user-facing arguments.
thanks @TomNicholas ! |