pik-copan / pyunicorn

Unified Complex Network and Recurrence Analysis Toolbox
http://pik-potsdam.de/~donges/pyunicorn/
Other
195 stars 87 forks source link

Clean up tutorial notebooks and documentation #213

Closed fkuehlein closed 7 months ago

fkuehlein commented 8 months ago

thereby resolving #185.

In bcc009d I updated the tutorial documentation, directly including rendered views of the notebooks using nbsphinx and nbsphinx-link, I think its quite a neat and cool way to make use of the notebooks. Any objections to these additional dependencies?

Note: nbsphinx currently throws a warning about an outdated pandoc version, which apparently is a known and resolved compatibility issue with nbconvert. Couldn't yet find out why it still throws the warning, but tox is passing anyway.

Also, I noticed the EventSeriesAnalysis tutorial is giving some unexpected results leading to division by zero warnings and such. (edit: resolved as of e19906e)

fkuehlein commented 8 months ago

Build is not being started... seems like we've already used up our credits at Travis again? @jdonges, could you try and find out? If so, maybe it would be worth considering GithubActions before we start setting up Windows and macOS builds...

fkuehlein commented 8 months ago

overwrote the last commit with a force-push of the identical changes, just to trigger a travis build. Seems to work again now as our credits have been re-stocked and we are welcome to ask for more any time!

fkuehlein commented 8 months ago

Build failed but somehow I don't have access to the build logs anymore, so I can't see why. If someone can and has a hint, let me know, otherwise will wait for my account to be granted full access to Travis!

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (f1b6e71) 62.82% compared to head (b351d31) 62.82%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #213 +/- ## ======================================= Coverage 62.82% 62.82% ======================================= Files 43 43 Lines 6220 6220 ======================================= Hits 3908 3908 Misses 2312 2312 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ntfrgl commented 8 months ago

@fkuehlein, please resolve the conflicts after merging #217. I will provide feedback afterwards; I guess via code comments, since this PR comes from your fork.

fkuehlein commented 8 months ago

Builds failed because in 1bd9ab8 I accidentally lost the additional dependencies under docs in the travis.yml. Windows and macOS, however, are reporting not to find pandoc although it is apparently installed. Will try adding it to travis.yml, which will probably not solve it for the Windows build, though. Read about some complications with python pandoc from pip here. @ntfrgl, any ideas on this? If it persists, maybe the notebook-inclusion is not worth all those additional dependencies..

ntfrgl commented 8 months ago

@fkuehlein, in order to iterate quickly, I would like to be able to push directly to this PR from your fork. This should be possible at first glance (Maintainers are allowed to edit this pull request), but in fact access is denied when I attempt it (remote: error: GH006: Protected branch update failed for refs/heads/master). Please either:


Regarding the problem in your previous message, I have a candidate solution which installs the Haskell executable pandoc on Windows via choco, while the Python wrapper pandoc remains in the Python dependency list. The conda approach, which we use on Linux and macOS, takes care of both.

fkuehlein commented 8 months ago

hi @ntfrgl, sorry for the late reply, I just removed the branch protection rule on fkuehlein:master. Let me know if you can push now, otherwise I will create a new one!

fkuehlein commented 7 months ago

On my local machine I found the problem to be with cartopy and its dependencies. I tried conda and pip installs of different versions, and finally found having cartopy >= 0.21 with an additional pyproj >= 3.4 in the setup.cfg to work, although throwing a DeprecationWarning (see commit above).

@ntfrgl, I hope this doesn't conflict with your reasoning of adding cartopy <= 0.21 ; python_version < 3.9 in aa39cf3? Also, we should probably document this additional dependency somewhere, for anyone who would want to use MapPlot with Python 3.8.

ntfrgl commented 7 months ago

Thank you for investigating this! I see, I had unintentionally over-constrained the micro version for cartopy on Python 3.8, which made the build miss https://github.com/SciTools/cartopy/pull/2110. I have now corrected the version constraints to capture the compatibility boundary with Python 3.8 more accurately.

fkuehlein commented 7 months ago

Thank you @ntfrgl for finalising! Also thanks again for reviewing the tutorials with some great cleanups and improvements. Glad to see this PR good to go.