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

Sentinel2-support #30

Merged
merged 3 commits into from
Jan 29, 2021
Merged

Sentinel2-support #30

merged 3 commits into from
Jan 29, 2021

Conversation

griembauer
Copy link
Contributor

This PR:

  • suggests to add Sentinel-2 (Level 1C) to available and downloadable datasets
  • swaps x and y in coordinate definition for correct handling (latitude fist)

@yannforget
Copy link
Owner

Many thanks for the contribution! Looks good to me. I'm gonna update the README and push a new release!

@yannforget yannforget merged commit efb9781 into yannforget:master Jan 29, 2021
@griembauer
Copy link
Contributor Author

Thanks a lot!

@@ -100,7 +100,7 @@ def search(
}

if latitude and longitude:
params.update(spatialFilter=spatial_filter(latitude, longitude))
params.update(spatialFilter=spatial_filter(longitude, latitude))
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC, this is swapped again: longitude is x axis, latitude is y. So, leave this as before (i.e., latitude, longitude) and change only the bbox order as the change implemented below, or only change this line to be longitude, latitude but leave the bbox as originally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Veronica, thanks for checking again, indeed I grew more and more confused with the x/y swapping ;-)
Let's do an example:

lon/lat only

Longitude 10.0°E, Latitude 50.0°N
landsatxplore search -u ... -p ... -d ... -s ... -e ... -l 50 10 (according to the readme, latitude comes first)
In cli.pyit gets into a dictionary:

    if location:
        latitude, longitude = location
        where.update(latitude=latitude, longitude=longitude) # so here latitude=50, longitude=10

Then, in api.search (api.py):

        if latitude and longitude:
            params.update(spatialFilter=spatial_filter(longitude, latitude))
# i.e.      params.update(spatialFilter=spatial_filter(10,50))

... and in datamodels.py:

def spatial_filter(xmin, ymin, xmax=None, ymax=None):
    if not xmax and not ymax:
        xmax = xmin + 0.1
        ymax = ymin + 0.1
    lower_left = coordinate(ymin, xmin) # coordinate(50,10)
    upper_right = coordinate(ymax, xmax) # coordinate(50.1,10.1)

# and coordinate is defined as
def coordinate(latitude, longitude):
    return {
        'latitude': latitude, # 'latitude': 50
        'longitude': longitude # 'longitude': 10
    }

This looks correct so far, now for a bounding box with xmin=10°E, ymin=50°N, xmax=11°E, ymax=51°N

bbox only

landsatxplore search -u ... -p ... -d ... -s ... -e ... -b 10 50 11 51 (according to the readme, x_min=longitude comes first here)
Then, in cli.py it is saved as bbox in this order:

    if bbox:
        where.update(bbox=bbox) # bbox = (10, 50, 11, 51)

then, in api.search (api.py):

        if bbox:
            params.update(spatialFilter=spatial_filter(*bbox)) # e.g. (spatial_filter(10, 50, 11, 51))

and in datamodels.py:

def spatial_filter(xmin, ymin, xmax=None, ymax=None):
    lower_left = coordinate(ymin, xmin) # coordinate(50,10)
    upper_right = coordinate(ymax, xmax) # coordinate(51,11)

...eventually

def coordinate(latitude, longitude):
    return {
        'latitude': latitude, # 'latitude': 50
        'longitude': longitude # 'longitude': 10
    }

So this looks correct too. I think most of the confusion comes from the fact that the -l flag takes latitude first and the -b flag takes longitude first. For my local testing with landsatxplore, all api-search results using both lat/lon or bbox made sense. But indeed I did not test it for i.landsat.download, as I simply assumed that since i.landsat.download depends on the bbox, which was changed in this PR, it would need to be adapted there too.

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.

3 participants