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

Clean up optional dependency groups #2541

Merged
merged 10 commits into from
Dec 16, 2024

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Dec 6, 2024

Fixes #2539.

@dstansby dstansby force-pushed the optional-deps branch 3 times, most recently from ffe3a95 to f464acb Compare December 6, 2024 14:55
Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

Thanks. Just one suggestion.

For groups with only one optional dependency, I think its best if we just use the dependency as the name.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@dstansby
Copy link
Contributor Author

dstansby commented Dec 7, 2024

Is there any benefit in having a dependency group where the name is the same as the package? I'd suggest we either

  • leave it as tree, to indicate "install this extra if you want to use tree()
  • remove it altogether and just document that folks need to install rich to use tree()

Copy link
Member

@jhamman jhamman left a comment

Choose a reason for hiding this comment

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

Is there any benefit in having a dependency group where the name is the same as the package? I'd suggest we either

  • leave it as tree, to indicate "install this extra if you want to use tree()

If we do this, we should adopt the "feature-oriented groups" approach for the other groups as well. That would apply to fsspec for RemoteStore.

  • remove it altogether and just document that folks need to install rich to use tree()

I favor this option actually for rich and universal-pathlib.

pyproject.toml Outdated Show resolved Hide resolved
@dstansby
Copy link
Contributor Author

dstansby commented Dec 9, 2024

pre-commit.ci autofix

@dstansby dstansby requested a review from jhamman December 9, 2024 10:53
@dstansby
Copy link
Contributor Author

dstansby commented Dec 9, 2024

If we do this, we should adopt the "feature-oriented groups" approach for the other groups as well

👍 done. I also removed universal-pathlib, tree, and started a v3 what's new (I don't think this already exists?) to communicate what's changed from v2.

@dstansby dstansby added this to the 3.0.0 milestone Dec 9, 2024
@dstansby dstansby enabled auto-merge (squash) December 16, 2024 12:26
@dstansby dstansby merged commit 77d0b11 into zarr-developers:main Dec 16, 2024
27 checks passed
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.

Clean up optional dependencies
2 participants