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

stage pekel water map #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oberonia78
Copy link
Contributor

This PR adds the new python script staging Pekel occurrence or seasonality map for given input coordinates.

Usage :
stage_pekel_water.py -b 177.38 -19.62 -178.88 -17.34 -s googleapi

@oberonia78 oberonia78 changed the title first draft for staging pekel water map stage pekel water map Sep 20, 2023
Copy link
Contributor

@seongsujeong seongsujeong left a comment

Choose a reason for hiding this comment

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

Thanks for adding the script to stage the water map. Here are my comments, suggestions, and questions - Seongsu

# [TODO] need to update the bucket
"""Name of the default S3 bucket containing the full JRC Pikel's water map to crop from.
"""
S3_REF_WATER_BUCKET = "opera-reference-water"
Copy link
Contributor

Choose a reason for hiding this comment

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

The commands below did not work for me. Can you provide me about more info about accessing the bucket?

aws s3 ls s3://opera-reference-water/
aws s3 ls --no-sign-request s3://opera-reference-water/

List of shapely polygons
epsgs: list, str
List of EPSG codes corresponding to
elements in polys
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add description for the return variable poly

"""Transform coordinates of polys (list of polygons)
to target epsgs (list of EPSG codes)

Parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Parameters:
Parameters

def get_geo_polygon(ref_tif):
"""Get polygon from GeoTiff image

Parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Parameters:
Parameters

ref_tif: str
Path to RTC product to stage the pekel's water map

Returns:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returns:
Returns

formatter_class=argparse.ArgumentDefaultsHelpFormatter
)
parser.add_argument('-p', '--product', type=str, action='store',
help='Input reference RTC HDF5 product')
Copy link
Contributor

Choose a reason for hiding this comment

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

I could be wrong, but it seems the function has changed to take in RTC GEOTIFF product to compute the polygon. Can you check?

Longitude margin as a result of the conversion
'''
delta_lon = (180 * 1000 * margin_in_km /
(np.pi * EARTH_RADIUS * np.cos(np.pi * lat / 180)))
Copy link
Contributor

Choose a reason for hiding this comment

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

what is going to happen when lat is 90.0 or very close to 90.0?

else:
logger.info(f"{url} not found")
counter += 1
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on what I see in here and check_aws_connection(),It looks like the code would only work with the google public storage. No need to add the functionality to make it work with S3 bucket, but wanted to check with you.

@seongsujeong
Copy link
Contributor

Couple of findings when testing the code:

  • Has encountered this error when running the script:

Symbol not found: _ZSTD_compressBound
Based on the information here, we might need to constrain the gdal >=3.5.1
The test was done on Mac after installing the dependencies by following the readme.

  • Had to install backoff separately. If you want to include this script into SAS, then I would suggest to add this conda package to requirements.txt

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