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

RTD: remove transitive deps from docs env #449

Merged
merged 3 commits into from
May 22, 2024

Conversation

mathause
Copy link
Member

This adds all of our direct dependencies to the docs environment (following #448) but removes the transitive ones. The direct ones were installed before, just not explicitly listed. Unfortunately sphinx (or maybe just the autosummary extension) imports the functions it wants to create the docs for, so they must be installed. However, leaving the optional transitive deps away should lead to a small speed up.


Alternatively we could install everything using pip but that would be more work (c.f. regionmask/regionmask#450)

Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.73%. Comparing base (9b0b76b) to head (2969028).
Report is 43 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #449      +/-   ##
==========================================
- Coverage   87.90%   83.73%   -4.18%     
==========================================
  Files          40       44       +4     
  Lines        1745     1949     +204     
==========================================
+ Hits         1534     1632      +98     
- Misses        211      317     +106     
Flag Coverage Δ
unittests 83.73% <ø> (-4.18%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@veni-vidi-vici-dormivi
Copy link
Collaborator

Ah another one.

Alternatively we could install everything using pip but that would be more work

Would that remove the need for the docs to have its own yml?

@mathause
Copy link
Member Author

mathause commented May 17, 2024

Potentially - we could use the docs extra for the installation, so python -m pip install .[docs]. That's another good reason for switching.

(As long as we don't need any non-python dependency [which was the ultimate problem in regionmask/regionmask#450])

@mathause
Copy link
Member Author

mathause commented May 17, 2024

I suggest to merge this and try the pip installation in another PR (see #453)

- statsmodels
- regionmask>=0.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we remove the version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Something less to keep track of. It should per default install the newest versions. (But yes there are also arguments for keeping it - so no real good reason).

@mathause mathause merged commit 7377946 into MESMER-group:main May 22, 2024
9 checks passed
@mathause mathause deleted the rtd_rm_transitive_deps branch May 22, 2024 09:37
veni-vidi-vici-dormivi pushed a commit to veni-vidi-vici-dormivi/mesmer that referenced this pull request May 23, 2024
* RTD: remove transitive deps from docs env

* add comment
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.

None yet

2 participants