allow UTM as --output-crs #59
Conversation
| @click.option( | ||
| "--output-crs", | ||
| required=False, | ||
| default="", |
There was a problem hiding this comment.
Do we need to change the default to be "utm" to support Aman's use-case? Or is it sufficient to just allow "UTM|utm" as a valid option and respond appropriately?
There was a problem hiding this comment.
I left blank to align with the rtc_s1 logic of no value meaning default zone, but I can change if clearer for users?
| burst_ids=() | ||
| resolution=20 | ||
| output_crs="" | ||
| output_crs="UTM" |
There was a problem hiding this comment.
Ah, I see that you now have this as the default here... in which case, it might be fine to leave the default as None in the cli.
sar_pipeline/aws/cli.py
Outdated
| # update the burst crs if it has been set | ||
| if output_crs and output_crs is not None: | ||
| # update the burst crs if it has been set and is not UTM | utm | ||
| if output_crs and (output_crs not in [None, "utm", "UTM", ""]): |
There was a problem hiding this comment.
I recommend if output_crs and (output_crs.lower() != "utm"): because if output_crs already takes care of output_crs being None or an empty string.
There was a problem hiding this comment.
I think the .lower() logic will raise an error if when integer is passed. I agree that None and "" are already covered so I can remove those
Co-authored-by: Caitlin Adams <[email protected]>
Co-authored-by: Caitlin Adams <[email protected]>
3 participants
No description provided.