-
Notifications
You must be signed in to change notification settings - Fork 54
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
Original time
encoding units not used in Zarr generated by beam-refactor
branch
#465
Labels
Comments
This is such a useful bug report. Thank you so much! 🙏 |
I have a potential fix for this almost working locally, I need to resolve some test failures. I'm hoping to create a PR later this week. |
Amazing! Yes, PR very welcome! |
derekocallaghan
added a commit
to derekocallaghan/pangeo-forge-recipes
that referenced
this issue
Jan 12, 2023
rabernat
pushed a commit
that referenced
this issue
Jan 18, 2023
* Original variable encodings are retained (#465) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Updated following @rabernat review comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Confirmed that this is resolved by #471. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While porting the EOOffshore CCMP recipe to use the
beam-refactor
branch, I noticed that the output Zarr contains non-uniquetime
values.It looks like the original dataset
time
encoding
units
aren't retained inpangeo_forge_recipes.writers._store_data()
:pangeo-forge-recipes/pangeo_forge_recipes/writers.py
Line 30 in eea8e44
At this point the original
time
encoding is as expected (i.e.hours...
):The
units
are subsequently replaced after this statement, where the use ofdays...
is resulting in the non-unique values:pangeo-forge-recipes/pangeo_forge_recipes/writers.py
Line 31 in eea8e44
I don't have a fix for this yet, but it looks like the issue is in
pangeo_forge_recipes.aggregation._to_variable()
based on the existing comments:pangeo-forge-recipes/pangeo_forge_recipes/aggregation.py
Lines 198 to 203 in eea8e44
The text was updated successfully, but these errors were encountered: