-
Notifications
You must be signed in to change notification settings - Fork 79
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
add consolidate to single_zarr #439
Conversation
kerchunk/zarr.py
Outdated
inline_threshold=100, | ||
inline=None, | ||
out=None, | ||
translate=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should just always be True, no need for the kwarg. (Also "translate" is the method on most scanning classes, so would be confusing).
A good argument to optionally include would be an output; at the moment we have out is None (return JSON), or a parquet set; if it were a .json path, we should write to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies pushed an old commit. renamed it to consolidate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look at the write to json
I think this is ready for review. I opened #441 to think about writing to json in the function a bit more |
No description provided.