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

Consolidate Metadata Transform #665

Merged
merged 9 commits into from
Jan 25, 2024

Conversation

norlandrhagen
Copy link
Contributor

@norlandrhagen norlandrhagen commented Jan 12, 2024

Update: Tests are passing for Zarr & Reference ConsolidateMetadata transform.
Update 2: Added to the PR an option for StoreToZarr to not consolidate the metadata automatically (It still defaults to consolidated_metadata).

@norlandrhagen norlandrhagen self-assigned this Jan 12, 2024
@norlandrhagen
Copy link
Contributor Author

Shoutout to @ranchodeluxe for the help with the tests!

@norlandrhagen
Copy link
Contributor Author

Update: Tests are passing for Zarr & Reference ConsolidateMetadata transform.

@norlandrhagen norlandrhagen changed the title [WIP] Consolidate Metadata Transform Consolidate Metadata Transform Jan 17, 2024
Copy link
Contributor

@ranchodeluxe ranchodeluxe left a comment

Choose a reason for hiding this comment

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

LGTM!

left a reply to remove the old comments regarding the past breaking change in consolidate_metadata

@norlandrhagen
Copy link
Contributor Author

It seems like regardless of if ConsolidateMetadata to the StoreToZarr pipeline, a .zmetadata is created. @cisaacstern any idea where this might be happening?

@ranchodeluxe
Copy link
Contributor

ranchodeluxe commented Jan 19, 2024

It seems like regardless of if ConsolidateMetadata to the StoreToZarr pipeline, a .zmetadata is created. @cisaacstern any idea where this might be happening?

A similar thing was happening in my PR when we are returning a fsspec.ReferenceFileSystem (so not exactly the same issue). This was the change needed (hope that helps with some ideas)

…e, instead of empty, which defaults to true. Kwarg also added so you can force StoreToZarr to write non-consolidated metadata
@norlandrhagen
Copy link
Contributor Author

Merging for now to unblock some Dev-Seed work.

cc @ranchodeluxe @abarciauskas-bgse @sharkinsspatial

@norlandrhagen norlandrhagen merged commit af3b80f into pangeo-forge:main Jan 25, 2024
@cisaacstern
Copy link
Member

Thanks all for pushing this forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants