-
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
[WIP - DRAFT] Ndpyramid #669
Conversation
Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Sharing some comments from a code review with @norlandrhagen.
Another note is that we should eventually look at memory usage on a local and flink runner
Default is None. | ||
:param pyramid_kwargs: Dict containing any kwargs that should be passed to ndpyramid. | ||
Default is None. | ||
|
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.
Eventually we should add tests for the two currently supported projections. Since this is only relevant for one of the methods, pyramid_reproject
I think it should go in pyramid_kwargs
rather than a separate kwarg.
Default is None. | ||
:param pyramid_kwargs: Dict containing any kwargs that should be passed to ndpyramid. | ||
Default is None. | ||
|
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.
Eventually it could be nice it include pyramid_method
to use coarsen
, regrid
, or reproject
rather than just reproject
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.
Good call! I updated the transform to include those methods and raise a NotImplimentedError for now if it's not reproject.
chunks = {"x": self.pixels_per_tile, "y": self.pixels_per_tile} | ||
if self.other_chunks is not None: | ||
chunks |= self.other_chunks | ||
|
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.
This is blocked by #520
pangeo_forge_recipes/transforms.py
Outdated
|
||
ds = xr.Dataset(attrs=attrs) | ||
|
||
ds.to_zarr(store=f"{self.target_root.root_path}/{self.store_name}") # noqa |
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.
Could compute=False be good to include here?
225a729
to
967e6eb
Compare
df47cb5
to
8e5b0cc
Compare
Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>
Closing this PR in favor or https://github.com/carbonplan/pangeo-forge-ndpyramid. |
Draft PR to explore generating pyramids via pangeo-forge-recipes. Uses ndpyarmid.