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.

Sign up for GitHub

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

Jump to bottom

Add Landsat collection 2, level 1 support #42

Merged
merged 14 commits into from Dec 13, 2020
Merged

Add Landsat collection 2, level 1 support #42

merged 14 commits into from Dec 13, 2020

Conversation

kylebarron
Copy link
Member

@kylebarron kylebarron commented Dec 13, 2020 •
edited

Closes #37

This is the second half of providing Landsat collection 2 support; the first half being #38.

kylebarron
kylebarron commented Dec 13, 2020
View reviewed changes
rio_tiler_pds/landsat/aws/landsat_collection2.py Outdated Show resolved Hide resolved
rio_tiler_pds/landsat/utils.py Outdated Show resolved Hide resolved
@kylebarron kylebarron marked this pull request as ready for review December 13, 2020 21:35
vincentsarago
vincentsarago reviewed Dec 13, 2020
View reviewed changes
rio_tiler_pds/landsat/utils.py Outdated
# S3 paths always use oli-tirs
_sensor_name = sensor_name
if _sensor_name in ["oli", "tirs"]:
_sensor_name = "oli-tirs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I'm referring to. There's no oli or tirs folder here

> aws s3 ls s3://usgs-landsat/collection02/level-1/standard/ --request-payer
                           PRE etm/
                           PRE mss/
                           PRE oli-tirs/
                           PRE tm/
2020-08-26 19:35:23       1313 catalog.json

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but you also said you didn't find any oli only scenes, right ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, and I added them to the tests
https://github.com/cogeotiff/rio-tiler-pds/pull/42/files#diff-be27e3477e1b5abde00cf4e303a5eeec6d0f57689c97a9f9c7468fe48262e3d8R178-R227

Plus a tirs-only scene
https://github.com/cogeotiff/rio-tiler-pds/pull/42/files#diff-be27e3477e1b5abde00cf4e303a5eeec6d0f57689c97a9f9c7468fe48262e3d8R278-R302

vincentsarago
vincentsarago reviewed Dec 13, 2020
View reviewed changes
rio_tiler_pds/landsat/utils.py Outdated
meta["sensor_name"] = sensor_name
meta["_sensor_name"] = _sensor_name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename _sensor_name to sensor_dir_name ? it's a bit confusing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about _sensor_s3_prefix? 😄

@kylebarron kylebarron merged commit 514bd46 into master Dec 13, 2020
@kylebarron kylebarron deleted the kyle/landsat-c2-l1 branch December 13, 2020 23:22
@vincentsarago vincentsarago mentioned this pull request Jan 5, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincentsarago vincentsarago vincentsarago approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add Landsat Collection 2
2 participants
@kylebarron @vincentsarago