search

ocean-eddy-cpt / gcm-filters

Diffusion-based Spatial Filtering of Gridded Data
https://gcm-filters.readthedocs.io/
Other
37 stars 24 forks source link

Optional dependencies to run notebooks #120

Closed NoraLoose closed 2 years ago

NoraLoose commented 2 years ago

Handles #118.

codecov-commenter commented 2 years ago

Codecov Report

Merging #120 (d824926) into master (7dfd3fe) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   98.41%   98.41%           
=======================================
  Files           9        9           
  Lines         944      944           
=======================================
  Hits          929      929           
  Misses         15       15
Flag Coverage Δ
unittests 98.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7dfd3fe...d824926. Read the comment docs.

NoraLoose commented 2 years ago

There seems to be a problem with conda-forge and either of the new packages added to docs/environment.yml. The same problem will arise when users try to install gcm-filters[examples] (see new [options.extras_require] section in setup.cfg). Converting this PR to draft again - not sure how to proceed, though.

NoraLoose commented 2 years ago

@jbusecke maybe you know how to proceed with this issue?

jbusecke commented 2 years ago

Unless conda does support this sort of syntax (a very quick search was unsuccessful for me) I think there are two options here:

  1. Add a specificenvironment_examples.yml file with everything needed to run
  2. Include the additional dependencies in the doc environment.

Generally I think 2 is nicer, if RTD can handle the additional install without too much delay/issues.

jbusecke commented 2 years ago

Ah sorry, I did not realize that the RTD build was indeed failing....

jbusecke commented 2 years ago

How about you try to use mamba instead of conda?

jbusecke commented 2 years ago

I am trying the switch over at https://github.com/xgcm/xgcm/pull/401. Once that works, Id be happy to implement this here.

NoraLoose commented 2 years ago

I ended up including the additional dependencies that are necessary to run the notebooks in docs/environment.yml. This seemed like the most straight-forward thing to do.

Maybe someone could look over this PR? It's the last issue that has to be addressed to get our package published in JOSS. 🎉

jbusecke commented 2 years ago

Thats usually my approach too. Looking at it now.

NoraLoose commented 2 years ago

Awesome, thanks @jbusecke! I agree that we should be consistent with conda vs. mamba. In the installation instructions (installation.rst), we just talk about conda now.