-
Notifications
You must be signed in to change notification settings - Fork 179
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 MultiBandReader and MultiBaseReader TilerFactories #230
Conversation
|
||
""" | ||
|
||
reader: Type[MultiBaseReader] = field() |
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.
reader is required
I think the code is fine. Something does feel a little bit off. I think it might be https://github.com/cogeotiff/rio-tiler/blob/master/rio_tiler/io/base.py#L440-L441. Its pretty weird to accept an optional argument and immediately throw an exception if that argument isn't present, but I do understand there were good reasons to make I think it would help to update MRO to add a |
…bands /assets endpoints
None, | ||
title="Asset indexes", | ||
description="comma (',') delimited asset names (might not be an available options of some readers)", | ||
..., title="Asset indexes", description="comma (',') delimited asset names.", |
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.
assets
is required
"""Return a list of supported assets.""" | ||
with rasterio.Env(**self.gdal_config): | ||
with self.reader(src_path.url, **self.reader_options) as src_dst: | ||
return src_dst.assets |
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 return the list of supported
assets by the tiler
**bands_params.kwargs, | ||
**metadata_params.kwargs, | ||
**kwargs, | ||
) |
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 add some tests for this Factory
@geospatial-jeff thanks for the review. I get your point but I think it might be a bit complicated to abstract the 2 factories into a baseclass and a subclass or a mixin. let's first merge this and then maybe open an issue :-) |
closes #228