Dependencies #73
Dependencies #73
Conversation
and comment the extras that aren't currently used in the tests
consistent with the conda-forge recipe
`ipython` is needed to syntax highlight the examples
in the nbsphinx nb parsing I don't recall getting these last time I built the docs, but whatever... The images need to have a unique alt. I was getting: WARNING: Duplicate substitution definition name: "image0" Note: using the filename as the alt in the markdown causes this warning: WARNING: Undefined substitution referenced: "movie_fast.gif" (and also the image is not shown)
if already have ffmpeg, don't need it if don't, can easily install with conda or other method
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
@jbusecke can you give this an initial look when you get a chance? |
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 78.52% 81.56% +3.03%
==========================================
Files 3 4 +1
Lines 326 358 +32
Branches 61 63 +2
==========================================
+ Hits 256 292 +36
+ Misses 46 43 -3
+ Partials 24 23 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Amazing start @zmoon. I briefly checked the changes, and think you are on a good track.
It was a bit hard to review this with the mix of formatting only changes, perhaps we should enforce the code formatting a bit more strictly in another PR?
I will definitely need to go over this one more time in more detail, but I made some suggestions that might help streamline the code. Let me know what you think
| @@ -72,6 +68,9 @@ def _base_plot(ax, base_data, timestamp, framedim, plotmethod=None, **kwargs): | |||
|
|
|||
| # projections utilities and hacks | |||
| def _smooth_boundary_NearsidePerspective(projection): | |||
| import cartopy.crs as ccrs | |||
There was a problem hiding this comment.
I am not sure this is the best way to do this. Could we define a decorator that checks if cartopy is available, and raises an error if not. Similar to what xarray is doing for their tests: https://github.com/pydata/xarray/blob/95bb9ae4233c16639682a532c14b26a3ea2728f3/xarray/tests/__init__.py#L43-L66
I might be missing something here, but this will not result in a nice error message for the user AFAIKT
There was a problem hiding this comment.
I did put such a message in the user-facing functions, e.g. here. But I like the decorator idea, to reduce duplication of code like this.
There was a problem hiding this comment.
@jbusecke I had a go at the decorator, let me know what you think.
|
@jbusecke thanks much for the suggestions. I will update the tests to use the xarray-style
Sorry about that; we could introduce a pre-commit config in another PR? |
That would be best I think. And no worries! I should have set this up earlier anyways. The growing pains of converting a pet project into a community maintainable package are real! |
|
Also we need to decide if we want to use the new pip requirements file I made for RTD, or stick with your mamba-forge solution that is currently working. |
|
@jbusecke there are still some things to resolve, but I think this is ready for another look |
|
Could you run pre-commit locally and rebase on the latest main? I would hope some of the formatting diffs would go away. Then Ill def go through it in more detail. Thanks for your patience on this. |
|
I tried to test for the # Separate file
import warnings
import pytest
from . import has_tqdm
def test_tqdm_message_on_import():
if has_tqdm:
with warnings.catch_warnings():
warnings.simplefilter("error")
import xmovie
else:
with pytest.warns(UserWarning, match="^Optional dependency `tqdm` not found."):
import xmoviebut it seems like it get emitted before where I am trying to catch. |
Closes #71 and also should speed up the RTD build
cartopyanddask.arraynow optionalMissingDependency)? to raise and change the tests to xfail on this instead of skipcartopyanddask-core?