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

Cleanup dependencies #798

Merged
merged 16 commits into from
Aug 23, 2021
Merged

Cleanup dependencies #798

merged 16 commits into from
Aug 23, 2021

Conversation

aulemahal
Copy link
Collaborator

@aulemahal aulemahal commented Aug 17, 2021

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
  • bumpversion (major / minor / patch) has been called on this branch
  • Tags have been pushed (git push --tags)
  • The relevant author information has been added to .zenodo.json

What kind of change does this PR introduce?

  • Removes all the "extra" dependencies from setup.py except "dev". All tools needed for testing, documenting and developping xclim are listed in requirements_dev.py.
  • Fixes a doctest issue triggered by cftime 1.5.
  • Removes the line that was installing cftime 1.4.1 in the test suite (!!!!)
  • Unpin pandas
  • Removes the mention of ".ipynb" as a source suffix in docs/conf.py. That variable, when it is a list, will associate all those files with the RST parser. Before nbsphinx 0.8.7 (10 Aug 2021), the plugin would override the current configuration, but now it doesn't, so the bug showed itself.

Does this PR introduce a breaking change?

I don't know if this count as a "breaking change", but installation options have changed.

Other information:

@Zeitsperre I removed some packages from the dev reqs:

ipykernel, ipython, jupyter_client, wheel, watchdog, twine

I was able to follow all the steps of the "contributing" page of the doc. Are any of those explicitely needed by developers nonetheless? I see that some are needed for packaging, but I think we can leave them out of the dev reqs, as only you or few others will ever do that task.

I haven't addressed #795, but could do it here if people agree with the idea.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Zeitsperre Zeitsperre self-requested a review August 18, 2021 13:37
Copy link
Collaborator

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

This is looking much nicer. Nice!

@Zeitsperre
Copy link
Collaborator

@Zeitsperre I removed some packages from the dev reqs:

ipykernel, ipython, jupyter_client, wheel, watchdog, twine

I was able to follow all the steps of the "contributing" page of the doc. Are any of those explicitely needed by developers nonetheless? I see that some are needed for packaging, but I think we can leave them out of the dev reqs, as only you or few others will ever do that task.

wheel and twine are only needed for uploading releases. Seeing as this is a major library of ours, it would be good to have the steps in the development docs. Feel free to keep them removed for now, but I'll open an issue to finish the packaging docs on another PR.

For the Jupyter-related libraries, we need a minimal environment in the dev recipe for running the notebooks via pytest at the very least. If we can get away without those libraries, great!

I had no idea watchdog was in there. Is that library used for some form of provenance tracking?

I haven't addressed #795, but could do it here if people agree with the idea.

I have some thoughts on this, but I'll add to the issue. My vote is to hold off for now.

@Zeitsperre
Copy link
Collaborator

Zeitsperre commented Aug 20, 2021

After our discussion from the other day, we'll go forward with the suggestion of bundling the testing dependencies into the GitHub source environment.yml (#795). Once that's in, we should be good to merge!

@aulemahal aulemahal merged commit abb4a0d into master Aug 23, 2021
@aulemahal aulemahal deleted the cleanup-dep-specs branch August 23, 2021 19:38
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.

Notebooks are not rendering in the docs unpin pandas<1.3
2 participants