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

[BUG] Added xarray to dependecies #116

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

s-weigand
Copy link

The notebook 07.01-ipyleaflet.ipynb uses xarray, which isn't part of the dependencies and thus Advanced example 1: Velocity can't be executed on binder.

IMHO adding netcdf4 to the normal dependencies and making the download code for the wind-global.nc dataset a code cell instead of an instruction in a markdown cell would be an improvement.

Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

Thanks, @s-weigand. Any chance you could also make the change you mentioned to the notebook? I think the reason we had it that way was, in part, that the hotel WiFi at SciPy, while good, crumbled under too much simultaneous load.

Not an issue when the conference is distributed!

Other tutorial folks, if this looks good to you too please go ahead and merge.

@s-weigand
Copy link
Author

Yeah WiFi at conferences ... 😆
And sure I can make the changes.
The only question would be if you would prefer the download code or just add the file and reference it, the source repo is under MIT license.

@mwcraig
Copy link
Contributor

mwcraig commented Aug 9, 2020

Let's go with the download code; some day we'll be able to be in person again and we don't want the repo to get too big.

and removed now obsolete download instructions. Also added 'wind-global.nc' to .gitignore, so it won't be uploaded accidentally.
so it is show in the output cell as well as in the Sidecar.
@s-weigand
Copy link
Author

I made the velocity map the last statement of the input cell, so it is shown in the output cell as well as in the Sidecar.
IMHO this is more consistent with the other examples, than just showing it in the Sidecar.

@s-weigand
Copy link
Author

Btw. I use Review Notebook App in projects where people collaborate on notebooks. This makes it easier for maintainers to spot differences in notebooks, since the can look at a rendered diff, instead of the json blob.

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.

2 participants