Split optional dependencies in pyproject.toml by TomNicholas · Pull Request #309 · zarr-developers/VirtualiZarr

TomNicholas · 2024-11-19T18:41:44Z

Takes the splitting of optional dependencies idea started in #87 to its logical conclusion.

Changes are documented in docs/releases.rst
New functionality has documentation

TomNicholas · 2024-11-19T18:42:41Z

+all = [
+    "virtualizarr[all_readers]",
+    "virtualizarr[icechunk]",


Right now I don't think this can possibly work, because icechunk requires zarr-python v3 but kerchunk requires <3.

not without truly living on the bleeding edge 😅

https://github.com/developmentseed/virtualize-nex-gddp-cmip6/blob/f331421ee87adcfb42cb4a1904c05ec1214eb932/pyproject.toml#L24

Not sure we should change the public pip install to use some random branch...

Aside: Is there a reason you are using virtualizarr main there? Would it be helpful to issue another release?

Not sure we should change the public pip install to use some random branch...

sorry, I wasn't suggesting you should use that approach, but just was sharing for information about what is/isn't possible. I should've been more clear.

Aside: Is there a reason you are using virtualizarr main there? Would it be helpful to issue another release?

I wanted strict type hinting (#306)

Right now I don't think this can possibly work, because icechunk requires zarr-python v3 but kerchunk requires <3.

Now that #412 is in this can work again, so we should push this PR forward too. But I'm not sure whether we should do this one before or after your Pixi PR #407 @maxrjones ?

TomNicholas

None of this is currently tested because the CI uses conda envs rather than pip to install deps. We could change that though (FYI @maxrjones).

However we don't really want one env and corresponding CI job for every single reader, it's fine to test them all at once, but that means we still wouldn't really be testing the ability to do pip install virtualizarr[netcdf3] specifically for example.

Also @norlandrhagen fastparquet is in the CI but doesn't seem to be used anywhere?

maxrjones · 2024-11-19T19:11:11Z

I could change the CI to use pip. I have been using uv a bit recently and it's been super promising for managing deps, but would be a bigger change.

cisaacstern · 2024-11-19T19:19:24Z

without much context, obligatory pixi endorsement comment

TomNicholas · 2024-11-19T19:43:55Z

I think the CI failure is because the mypy CI job is apparently the only one that uses pip to install currently

norlandrhagen · 2024-11-19T19:58:19Z

Also @norlandrhagen fastparquet is in the CI but doesn't seem to be used anywhere?

I think it's a hidden requirement of using the Kerchunk parquet format.

TomNicholas · 2024-11-19T20:06:30Z

I think it's a hidden requirement of using the Kerchunk parquet format.

So does that mean we should have another writer split out in these dependencies?

maxrjones · 2024-11-19T20:57:46Z

without much context, obligatory pixi endorsement comment

emerging from a rabbit hole (thanks @cisaacstern 😂) I'm floating around this decision framework for feedback:

If you're starting a new Python package, use uv
If you need to manage multiple Python virtual environments at a time, also use hatch
If you need Conda-Forge in addition to PyPI distributions, ~~also~~ use pixi

More on topic, feel welcome to ping me if you'd like to assign any CI / packaging updates.

norlandrhagen · 2024-11-19T22:23:39Z

So does that mean we should have another writer split out in these dependencies?

Probably! Also, maybe for reading existing Kerchunk parquet refs.

TomNicholas · 2024-11-20T15:15:05Z

Probably! Also, maybe for reading existing Kerchunk parquet refs.

Does reading existing Kerchunk parquet refs require the same additional dependencies? If so we could have kerchunk_format = ["fastparquet"] as another group?

norlandrhagen · 2024-11-20T17:20:49Z

Does reading existing Kerchunk parquet refs require the same additional dependencies? If so we could have kerchunk_format = ["fastparquet"] as another group?

I just checked, both reading the writing of Kerchunk Parquet refs require fastparquet.

for more information, see https://pre-commit.ci

TomNicholas · 2024-11-21T00:23:15Z

In a633a0c I embedded the pyproject.toml into the installation docs page so that the list of pip-installable optional dependencies always stays up-to-date. See

https://virtualizarr--309.org.readthedocs.build/en/309/installation.html

TomNicholas · 2024-12-17T16:56:13Z

note to self: add various parallel options to the splitting here once #349 is in, because that adds optional dependencies on lithops and dask

maxrjones · 2025-02-03T15:47:54Z

FYI there's some redundancy between this PR and #407

TomNicholas · 2025-02-03T16:24:03Z

FYI there's some redundancy between this PR and #407

Yep - which one shall we try to merge first? I think this one can be done very quickly, but I don't know if it will fully work without what you're doing in #407?

maxrjones · 2025-02-03T16:33:39Z

FYI there's some redundancy between this PR and #407

Yep - which one shall we try to merge first? I think this one can be done very quickly, but I don't know if it will fully work without what you're doing in #407?

I recommend merging this first. It should not depend on anything in #407.

codecov · 2025-02-03T20:27:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.01%. Comparing base (14f58ba) to head (d79e348).
Report is 31 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #309      +/-   ##
==========================================
- Coverage   84.17%   84.01%   -0.17%     
==========================================
  Files          31       31              
  Lines        1839     1839              
==========================================
- Hits         1548     1545       -3     
- Misses        291      294       +3

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>

TomNicholas · 2025-02-03T21:26:28Z

+# un-implemented readers
+# tiff = [
+#     "virtualizarr[remote]",
+#     "kerchunk>=0.2.8",
+#     "tifffile",
+# ]
+# grib = [
+#     "virtualizarr[remote]",
+#     "kerchunk>=0.2.8",
+# ]


These commented-out lines are going to appear in the docs - https://virtualizarr--309.org.readthedocs.build/en/309/installation.html#optional-dependencies.

Should I (a) remove them or (b) add issue links to encourage people to contribute those readers?

I think we should remove them because I don't think it's a certainty that those readers will be built based on kerchunk and its dependencies. e.g., I have it on my roadmap for the next 2 months to implement a reader based on https://github.com/developmentseed/aiocogeo-rs/.

TomNicholas added 2 commits November 19, 2024 13:38

split optional deps

6d87930

comment out un-implemented readers

5b14477

TomNicholas added Packaging dependencies Updates a dependency labels Nov 19, 2024

TomNicholas temporarily deployed to test-release November 19, 2024 18:42 — with GitHub Actions Inactive

TomNicholas commented Nov 19, 2024

View reviewed changes

remove _reader suffix

850c38a

TomNicholas temporarily deployed to test-release November 19, 2024 18:49 — with GitHub Actions Inactive

TomNicholas mentioned this pull request Nov 20, 2024

Use uv / pixi? #310

Closed

add kerchunk as writer to cover fastparquet dependency

aeecf8a

TomNicholas temporarily deployed to test-release November 20, 2024 20:58 — with GitHub Actions Inactive

TomNicholas and others added 5 commits November 20, 2024 16:15

Merge branch 'main' into split_optional_deps

e1df506

embed the pyproject.toml in the installation docs

a633a0c

notes on test and docs deps

4d77fa1

wording

32966e5

[pre-commit.ci] auto fixes from pre-commit.com hooks

aa34761

for more information, see https://pre-commit.ci

pre-commit-ci Bot temporarily deployed to test-release November 20, 2024 22:18 Inactive

release note

cc8a218

TomNicholas temporarily deployed to test-release November 20, 2024 22:20 — with GitHub Actions Inactive

Merge branch 'main' into split_optional_deps

b4cfe5c

TomNicholas temporarily deployed to test-release December 3, 2024 22:49 — with GitHub Actions Inactive

TomNicholas added 3 commits February 3, 2025 12:57

Merge branch 'main' into split_optional_deps

28cbb03

update kerchunk version

2cbcd2f

separate remote options

af52c4c

TomNicholas temporarily deployed to test-release February 3, 2025 20:26 — with GitHub Actions Inactive

remove sneaky git thing

65c4af1

TomNicholas temporarily deployed to test-release February 3, 2025 20:27 — with GitHub Actions Inactive

maxrjones reviewed Feb 3, 2025

View reviewed changes

Comment thread pyproject.toml Outdated

Update pyproject.toml

a81a845

Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>

TomNicholas temporarily deployed to test-release February 3, 2025 21:24 — with GitHub Actions Inactive

update kerchunk verion

a34843b

TomNicholas temporarily deployed to test-release February 3, 2025 21:25 — with GitHub Actions Inactive

TomNicholas commented Feb 3, 2025

View reviewed changes

remove commented-out readers

d79e348

TomNicholas temporarily deployed to test-release February 3, 2025 22:12 — with GitHub Actions Inactive

TomNicholas merged commit 24a4582 into main Feb 3, 2025

TomNicholas deleted the split_optional_deps branch February 3, 2025 22:38

Uh oh!

Conversation

TomNicholas commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomNicholas Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

maxrjones Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

TomNicholas Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

maxrjones Nov 19, 2024

Choose a reason for hiding this comment

Uh oh!

TomNicholas Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

TomNicholas left a comment

Choose a reason for hiding this comment

Uh oh!

maxrjones commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cisaacstern commented Nov 19, 2024

Uh oh!

TomNicholas commented Nov 19, 2024

Uh oh!

norlandrhagen commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomNicholas commented Nov 19, 2024

Uh oh!

maxrjones commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

norlandrhagen commented Nov 19, 2024

Uh oh!

TomNicholas commented Nov 20, 2024

Uh oh!

norlandrhagen commented Nov 20, 2024

Uh oh!

TomNicholas commented Nov 21, 2024

Uh oh!

TomNicholas commented Dec 17, 2024

Uh oh!

maxrjones commented Feb 3, 2025

Uh oh!

TomNicholas commented Feb 3, 2025

Uh oh!

maxrjones commented Feb 3, 2025

Uh oh!

codecov Bot commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

TomNicholas Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

maxrjones Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TomNicholas commented Nov 19, 2024 •

edited

Loading

maxrjones commented Nov 19, 2024 •

edited

Loading

norlandrhagen commented Nov 19, 2024 •

edited

Loading

maxrjones commented Nov 19, 2024 •

edited

Loading

codecov Bot commented Feb 3, 2025 •

edited

Loading