Skip to content
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

Merged
merged 3 commits into from
Jul 12, 2019
Merged

Improve open_mfdataset deprecation warnings #3101

merged 3 commits into from
Jul 12, 2019

Conversation

TomNicholas
Copy link
Member

@pep8speaks
Copy link

pep8speaks commented Jul 12, 2019

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
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #3101 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            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

@dcherian
Copy link
Contributor

Awesome. I think this looks great. Sorry for not catching it earlier.

@max-sixty
Copy link
Collaborator

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,
Copy link
Member

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.

@shoyer shoyer merged commit ed421e8 into pydata:master Jul 12, 2019
@shoyer
Copy link
Member

shoyer commented Jul 12, 2019

thanks @TomNicholas !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

auto_combine warnings with open_mfdataset
5 participants