feat: Single-COG/item reads on native grid - #79
Conversation
hrodmn
left a comment
There was a problem hiding this comment.
@Berhinj thanks for opening this! The open_cog/open_cog_async methods will be useful. Looking at your example notebook makes me think that an open_item method would also be useful where you provide an entire STAC item and a list of bands. We would need to check the extent/shape of each geotiff asset to make sure they are equal but it could be a nice complement to the open_cog option for cases where there are multiple bands.
- Introduced a new helper function `_write_synthetic_cog` to streamline the creation of synthetic COGs with customizable parameters such as size, native resolution, and band count. - Updated the `synthetic_cog` fixture to utilize the new helper function, maintaining the original properties while allowing for more flexibility. - Introduction open_item - Undo lock and pyproject changes
|
@hrodmn thanks for your interest, I've put on the work and done a clean implementation now, tell me what you think |
| reprojection. Source ``nodata`` is set as ``_FillValue`` and any | ||
| ``scale``/``offset`` as ``scale_factor``/``add_offset`` so rioxarray's | ||
| ``mask_and_scale`` decoding applies them. |
There was a problem hiding this comment.
Can check with @hrodmn but I think this project is using Markdown formatting, which means you only need one backtick. Did AI write these docs?
There was a problem hiding this comment.
Yes I rely a lot on on claude opus as everyone for routines, especially for docstring and tests, for code like this one I still wrote a good chunk myself and made sure to review all ai written code, it is true though that I'm spending as much effort on reviewing ai written docstring
There was a problem hiding this comment.
regarding the backtick, indeed in markdown it is single tick but in docstring it is two, tbh I copied the existing style from the other .py files you have which is already using double tick, example: https://github.com/Berhinj/lazycogs/blob/47ae2b6e0c43db3db7d899fc7e564e7746a86f8f/scripts/prepare_benchmark_data.py#L69-L70
There was a problem hiding this comment.
I'm not worried about the docstring formatting in the context of this PR. All of the existing docstrings were written by AI and most of them are too verbose anyways. I'll open an issue to track a cleanup.
| ) -> DataArray: | ||
| """Open one COG at native resolution as an ``(band, y, x)`` DataArray. | ||
|
|
||
| Async variant of :func:`open_cog` for use inside a running event loop. |
There was a problem hiding this comment.
Ditto we're not using Sphinx docstring formatting in this project
There was a problem hiding this comment.
correct, it is not compatible with mkdocs, I was copying the style from the existing functions of the repo - example: https://github.com/Berhinj/lazycogs/blob/47ae2b6e0c43db3db7d899fc7e564e7746a86f8f/src/lazycogs/_grid.py#L55
happy to correct it but the rest of the repo is full of it, could we keep it in a seperate pr?
|
@kylebarron your comments on docstring formatting are valid but they're everywhere in the repo, it is an existing issue, I believe they should be addressed in a different PR globally instead - which I did here #87 |
|
@Berhinj thanks for working on this feature! I was out of office the last two weeks but will take a look soon. |
Motivation
I believe people would benefit from loading a single scene untouched on original grid, reusing lazycogs' existing pieces but still getting an xarray out of it.
Summary
Adds
lazycogs.open_cogandlazycogs.open_itemto read a single COG or STAC item at its native grid (no reprojection), reusing existingobstore/async-geotiff/rustaccomponents.