Repository navigation
Conversation
Version 1.3.2 includes range bandwidth (`bandwidth`, in GHz), relative burst numbers (`relative_burst`, starting at 1), beam identifiers (`beam_ids`), and compact polarizations (`LH`, `LV`, `RH`, `RV`, `CH`, and `CV`). `sar:product_type` is deprecated in favor of `product:type`, and `sar:instrument_mode` is deprecated in favor of `instrument_modes` from the instruments extension. Both remain available for compatibility. Co-authored-by: Fabrice Brito <fabrice.brito@terradue.com> Spec: https://stac-extensions.github.io/sar/v1.3.2/schema.json
bmcandr
left a comment
bmcandr
left a comment
There was a problem hiding this comment.
hey @sim13pods, thanks for your contribution! Overall I think this looks great. Just two minor changes requested.
| Deprecated in SAR v1.3.2 in favor of ``instrument_modes`` from the | ||
| instruments extension. | ||
| """ | ||
| return self._get_property(INSTRUMENT_MODE_PROP, str) |
There was a problem hiding this comment.
@gadomski what's your take on raising deprecation warnings in accessors for props? Existing deprecation warnings in the repo signal deprecations in the API, not the spec, and these accessors will stick around for a while. Just wondering if it's worth warning about, maybe in the setter, to nudge users towards current/future extension spec?
There was a problem hiding this comment.
Thanks @bmcandr! My preference would be to document the spec deprecations in the accessors for now, while keeping their runtime behavior unchanged. These accessors remain supported for compatibility, and a DeprecationWarning could suggest that the Python API itself is scheduled for removal.
If we do want to nudge users toward the replacement fields, I agree that the setters would be the better place. Reading existing metadata should remain quiet, and assigning None to remove a deprecated field should not warn either. We could limit warnings to assignments of non-None values and explicitly identify the replacement: product:type for sar:product_type, and instrument_modes for sar:instrument_mode.
That distinction also matters for apply(), which calls these setters even when the optional arguments are omitted.
@gadomski, happy to follow whichever policy you think fits PySTAC best. It would be useful to apply the same approach consistently across extensions.
This branch has not been deployed
2 participants
Related Issue(s):
Description:
Version 1.3.2 includes range bandwidth (
bandwidth, in GHz), relative burst numbers (relative_burst, starting at 1), beam identifiers (beam_ids), and compact polarizations (LH,LV,RH,RV,CH, andCV).sar:product_typeis deprecated in favor ofproduct:type, andsar:instrument_modeis deprecated in favor ofinstrument_modesfrom the instruments extension. Both remain available for compatibility.Spec: https://stac-extensions.github.io/sar/v1.3.2/schema.json
PR Checklist:
pre-commit run --all-files)pytest)