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

Include optional dependencies from dtscalibration dependencies #186

Merged
merged 7 commits into from
Jul 30, 2023

Conversation

bdestombe
Copy link
Collaborator

Use "xarray[parallel,accel]" instead of filling up the dtscalibration dependency list with optional xarray dependencies.

Use "xarray[parallel,accel]" instead of filling up the dtscalibration dependency list with optional xarray dependencies.
@BSchilperoort
Copy link
Collaborator

I see your branch is out of date. Want to first handle off #187 and then redo this PR? 😇

@bdestombe
Copy link
Collaborator Author

I see your branch is out of date. Want to first handle off #187 and then redo this PR? 😇

Sure! No rush

@BSchilperoort
Copy link
Collaborator

It would make for a cleaner commit (just a single commit to main, by using the rebase and merge option) if you would create a new branch from main and redo the changes on there.

Seeing as we directly import from dask, I would leave that a direct dependency. Same for pandas. Things we don't directly import (flox) can be installed with the extras options for xarray.

As it only used by statsmodels, and now included in the statsmodels dependencies.
It is not directly used by our scripts and cannot identify it as an optional dependency of one of our dependencies
Copy link
Collaborator

@BSchilperoort BSchilperoort left a comment

Choose a reason for hiding this comment

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

Looks good to me

@BSchilperoort BSchilperoort merged commit 071ae22 into main Jul 30, 2023
11 checks passed
@BSchilperoort BSchilperoort deleted the optional-xarray-dependencies branch July 30, 2023 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants