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

Ecosystem section #123

Merged
merged 14 commits into from
Jul 8, 2022
Merged

Ecosystem section #123

merged 14 commits into from
Jul 8, 2022

Conversation

TomNicholas
Copy link
Member

@TomNicholas TomNicholas commented Jul 6, 2022

Intended to fill part of the Ecosystem section of the xarray scipy tutorial, so addressing #116.

Planning to have a short bit of material on each of these topics:

  • Flexibility of xarray
  • Accessors as a concept
  • hvplot accessor
  • Rioxarray
  • cf-xarray
  • pint-xarray
  • defining your own accessors
  • Wider xarray ecosystem

Currently a work-in-progress.

@dcherian @scottyhq I'm planning to try and write the hvplot, pint-xarray, and possibly cf-xarray sections now, but I'm really not at all familiar with rioxarray (I've never even imported it 😅). I'm happy to present all this content in person on the day, but I think it would be much quicker if one of you guys added the section on rioxarray.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@scottyhq scottyhq left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @TomNicholas !

I'm happy to present all this content in person on the day, but I think it would be much quicker if one of you guys added the section on rioxarray.

Sounds good. Feel free to leave a blank cell and I'll follow up after this is merged with a tiny example.

@TomNicholas
Copy link
Member Author

I think this is basically ready to merge, just the rioxarray section missing (plus how did you get your exercises to be in drop-down boxes?)

@scottyhq
Copy link
Contributor

scottyhq commented Jul 8, 2022

plus how did you get your exercises to be in drop-down boxes?

would be good to normalize exercise formatting across notebooks (#88), for now if you want a dropdown on the rendered version you add hide-input or hide-output metadata tags https://jupyterbook.org/en/stable/content/metadata.html

@scottyhq
Copy link
Contributor

scottyhq commented Jul 8, 2022

looks great @TomNicholas, just need to import xarray as xr at the top! feel free to merge if you're happy with it

@TomNicholas
Copy link
Member Author

Okay great @scottyhq - I've added an import xarray as xr at the top, so I think we should just merge this content now and fix minor formatting gripes later.

@TomNicholas
Copy link
Member Author

Should I be worried about the build-and-deploy test failing?

@scottyhq
Copy link
Contributor

scottyhq commented Jul 8, 2022

Oh, right, you can see the traceback in the 'Dump build logs' step:

nbclient.exceptions.CellExecutionError: An error occurred while executing the following cell:
[25](https://github.com/xarray-contrib/xarray-tutorial/runs/7256684119?check_suite_focus=true#step:6:26)
------------------
[26](https://github.com/xarray-contrib/xarray-tutorial/runs/7256684119?check_suite_focus=true#step:6:27)
d + t

If you add a raises-exception tag to that cell, jupyterbook knows the error is anticipated and will execute subsequent cells https://jupyterbook.org/en/stable/content/execute.html#dealing-with-code-that-raises-errors

@dcherian what do you think of adding a branch protection rule to require that test to pass before merging? i don't have that admin access it seems

@dcherian

This comment was marked as duplicate.

@dcherian
Copy link
Contributor

dcherian commented Jul 8, 2022

what do you think of adding a branch protection rule to require that test to pass before merging? i don't have that admin access it seems

Just enabled!

@TomNicholas TomNicholas merged commit 2446b4a into xarray-contrib:main Jul 8, 2022
@TomNicholas TomNicholas deleted the ecosystem branch July 8, 2022 22:27
@@ -10,26 +10,126 @@
"\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Line #2.    import cf_xarray.units  # must be imported before pint_xarray

the import order is not important anymore with recent versions of pint, so you should probably remove the comment


Reply via ReviewNB

@scottyhq scottyhq mentioned this pull request Jul 9, 2022
@scottyhq
Copy link
Contributor

scottyhq commented Jul 9, 2022

sorry @keewis i remember you pointed that out before. i removed the comment in #128

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.

4 participants