-
Notifications
You must be signed in to change notification settings - Fork 2
Add ability to reproject products #29
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
Conversation
…g, db scaling, or both
| new_ymax = min(bounds.ymax + buffer[1], 90) | ||
| new_xmin = max(bounds.xmin - buffer[0], -180.0) | ||
| new_ymin = max(bounds.ymin - buffer[1], -90.0) | ||
| new_xmax = min(bounds.xmax + buffer[0], 180 - 0.5 * lon_spacing) |
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.
Just checking this logic is required?
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.
Looks good to me! Just confirming a couple things and we should make a decision about CRS formatting
sar_pipeline/nci/cli.py
Outdated
| @click.option("--spacing", type=int) | ||
| @click.option("--scaling", type=click.Choice(["linear", "db"])) | ||
| @click.option("--scaling", type=click.Choice(["linear", "db", "both"])) | ||
| @click.option("--target-crs", type=click.Choice(["EPSG:4326", "EPSG:3031"])) |
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 think we need to be consistent across the project (and other projects). I have typically passed in crs as an integer value and assumed EPSG:{crs}. This might not be best practice but would align with what I have done elsewhere.
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 think that's sensible -- I'm happy to update mine to match. I agree that consistency is important.
sar_pipeline/nci/cli.py
Outdated
| click.echo("Performing reprojection to EPSG:3031") | ||
| # Identify all files containing gamma0-rtc_geo | ||
| files_to_reproject = list( | ||
| processed_scene_directory.glob("*gamma0-rtc_geo*.tif") |
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.
Confirming this consider all required ancillaries?
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.
Ah, no it doesn't. I only focussed on getting the polarisation measurements, and didn't consider the ancillaries. I will go back and update.
…ucts Add ability to reproject products
Using the Copernicus DEM results in georeferenced products in EPSG:4326. This adds the ability to specify EPSG:3031 as the output and uses gdal to do the transform. Also adds the ability to select linear, dB or both as the scaling.