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

Pass zarr encoding through StoreToZarr() #674

Merged
merged 3 commits into from
Jan 29, 2024

Conversation

thodson-usgs
Copy link
Contributor

@thodson-usgs thodson-usgs commented Jan 24, 2024

This PR will allow passing ds.to_zarr() encodings to StoreToZarr(), which would allow custom compressors in recipes.
However, I'm hitting an error with my test recipe, which has me stumped for the moment...

line 49, in _store_data
    raise ValueError(
ValueError: Region (slice(0, 1, None),) does not align with Zarr chunks (2,). [while running 'Create|OpenURLWithFSSpec|OpenWithXarray|StoreToZarr/StoreToZarr/StoreDatasetFragments/Map(store_dataset_fragment)']

Here's my test recipe.
On the recommendation of @rabernat, I was attempting to use pcodec in StoreToZarr(), but for now, I'm testing with
zarr.Blosc(cname="zstd", clevel=3).

#!/bin/bash
REPO=https://github.com/hytest-feedstocks/gpcp-pcodec-feedstock.git
CONFIG_FILE=config.py
RECIPE_ID=gpcp-from-gcs
REF=recipe-dev
JOB_NAME=test

pangeo-forge-runner bake --repo=$REPO -f=$CONFIG_FILE --Bake.recipe_id=$RECIPE_ID --Bake.ref=$REF --Bake.job_name=$JOB_NAME --prune

Any ideas?

@thodson-usgs thodson-usgs marked this pull request as draft January 24, 2024 21:40
@thodson-usgs thodson-usgs force-pushed the add-custom-compression branch from fd3639f to 6110cda Compare January 25, 2024 20:53
@thodson-usgs
Copy link
Contributor Author

thodson-usgs commented Jan 26, 2024

... I'm an idiot. My recipe was trying to compress "time" not "precip". No wonder.

I've tested this on local runner and the compression works. Now to write a test.

@thodson-usgs thodson-usgs changed the title WIP: Pass ds.to_zarr() kwargs through StoreToZarr() Pass zarr encoding through StoreToZarr() Jan 26, 2024
@thodson-usgs thodson-usgs marked this pull request as ready for review January 26, 2024 13:33
@cisaacstern cisaacstern added the test-integration Apply this label to run integration tests on a PR. label Jan 26, 2024
@cisaacstern
Copy link
Member

Thanks so much @thodson-usgs, just approved the tests! Generally LGTM!

@cisaacstern
Copy link
Member

All tests passing, so I'll merge, thanks so much @thodson-usgs !

@cisaacstern cisaacstern merged commit 9498991 into pangeo-forge:main Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-integration Apply this label to run integration tests on a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants