-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Xarray and Dask Example #51
Conversation
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.
Finally I might be able to better understand Xarray (OK, did not took the time to dive in it yet).
Nice the PR with integrated binder!
Some problem here though, seems like the Notebook won't use Dask, not sure why. I did start the LocalCluster though. Maybe something in load_dataset?
This is ready for a proper review. Tests were failing earlier due to some churn leading up to the 0.11 release of xarray. That's all behind us now and things are now in a good place. |
I'll take another look at this this afternoon for style consistency. Is there anyone who should review this on the Xarray side? |
Some micro-feedback:
These are small bits of feedback though. In general I'm more than happy to merge this as-is. |
@mrocklin - I'll address most of these. Yes, the |
I think it's totally fine to use da if it's conventional for xarray. I was
just checking.
…On Fri, Nov 9, 2018 at 1:30 PM Joe Hamman ***@***.***> wrote:
@mrocklin <https://github.com/mrocklin> - I'll address most of these.
Yes, the da pronoun is common in xarray (da = DataArray()). This follows
the standard usage of df for Pandas' DataFrame. I now this conflicts with
dask.array so I'll adjust this accordingly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASszNRNqYxW0_06ZPLxa78UyhKo7HTMks5utcmxgaJpZM4YM4Qt>
.
|
I addressed all of your comments, except the |
Merging tomorrow if there are no further comments from xarray folks
…On Fri, Nov 9, 2018 at 5:34 PM Joe Hamman ***@***.***> wrote:
I addressed all of your comments, except the da one which I think we've
agreed to let slide.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASszMVKLRDJAVKmj5467nWfWCRlHrqGks5utgLvgaJpZM4YM4Qt>
.
|
Okay, @mrocklin - this is good to go. |
Thanks for setting this up @jhamman ! |
cc @mrocklin