Skip to content
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

Adding default_assets for MultiBaseReader and STACReader #722

Merged
merged 6 commits into from
Sep 9, 2024

Conversation

mccarthyryanc
Copy link
Contributor

@mccarthyryanc mccarthyryanc commented Aug 23, 2024

I have a specific use case when using titiler where I can't pass assets to the STACReader methods. As a solution to this, I added an optional default_assets attribute to be used like:

with STACReader(STAC_PATH, default_assets=["green"]) as stac:
    img = stac.tile(71, 102, 8)

PR includes tests and updates to docs.

ref: #720

@vincentsarago
Copy link
Member

@mccarthyryanc I'll try to review/merge this PR this week.

Quick question, are you planning to use this with TiTiler? right now this won't work without customizing the Dependencies because the default dependencies expect assets/expression to be set or it will raise a 500 errors. At runtime the dependencies can check if the item as default_assets set so this will be pretty hard to enable dynamic check.

Note: this could be resolved using the Render extension at a collection level

rio_tiler/io/base.py Outdated Show resolved Hide resolved
rio_tiler/io/stac.py Outdated Show resolved Hide resolved
@mccarthyryanc
Copy link
Contributor Author

Thanks for the review!

Quick question, are you planning to use this with TiTiler? right now this won't work without customizing the Dependencies because the default dependencies expect assets/expression to be set or it will raise a 500 errors. At runtime the dependencies can check if the item as default_assets set so this will be pretty hard to enable dynamic check.

Note: this could be resolved using the Render extension at a collection level

Yes, I'm using this with titiler. I already needed custom dependencies for some other inputs so it was simple enough to add in the default_assets.

@vincentsarago
Copy link
Member

vincentsarago commented Sep 9, 2024

@mccarthyryanc thanks for the PR, I've made some edits:

  • raise warnings for all the methods (it was implemented only for tile())
  • add default_bands to the MultiBandReader

let's hope no one comes with the need of default_expression 😅

I'll wait for your 👍 before merging

@mccarthyryanc
Copy link
Contributor Author

Awesome! Thanks for the changes @vincentsarago, those all look great!

@vincentsarago vincentsarago merged commit 9bd2127 into cogeotiff:main Sep 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants