add MultiAssetsReader base class and refactor STACReader #225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
vincentsarago
commented
Aug 12, 2020
| "center": self.center, | ||
| "minzoom": self.minzoom, | ||
| "maxzoom": self.maxzoom, | ||
| } |
There was a problem hiding this comment.
Before we were creating a Class with COGReader as parent so both center and spatial_info existed. I added those back for compatibility
vincentsarago
commented
Aug 12, 2020
| for (t, m), asset in _filter_tasks(tasks): | ||
| tasks = create_tasks(reader, chunks, threads, *args, **kwargs) | ||
| for (t, m), asset in filter_tasks( | ||
| tasks, allowed_exceptions=(TileOutsideBounds,) |
There was a problem hiding this comment.
ref #224, we allow TileOutsideBounds errors to be passed, they will still be logged
vincentsarago
commented
Aug 12, 2020
| for asset in assets | ||
| ] | ||
| else: | ||
| return ((reader(asset, *args, **kwargs), asset) for asset in assets) |
There was a problem hiding this comment.
New submodule because those functions are not just used in mosaic now
vincentsarago
commented
Aug 12, 2020
kylebarron
reviewed
Aug 15, 2020
vincentsarago
commented
Aug 17, 2020
rio_tiler/io/base.py
Outdated
| if not tile_exists(self.bounds, tile_z, tile_x, tile_y): | ||
| raise TileOutsideBounds( | ||
| "Tile {}/{}/{} is outside image bounds".format(tile_z, tile_x, tile_y) | ||
| ) |
There was a problem hiding this comment.
I think we should remove this. Checking if the tile is outside the bounds here lets us raise before doing more file reading BUT it makes the class not really extendable for rio-tiler-crs
kylebarron
approved these changes
Aug 18, 2020
kylebarron
reviewed
Aug 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #224
closes #213
In addition to ☝️, this PR adds a new Abstract base for Multiple Assets merging (mainly for rio-tiler-pds but also used in STACReader). With this, we can specify any kind of Reader within the
MultiAssetsReader. Before we were using therio_tiler.io.cogeo.multi_*methods tied to COGReader.cc @kylebarron @geospatial-jeff