-
Notifications
You must be signed in to change notification settings - Fork 51
fix: asset name harmonisation for S1_SAR_GRD #1983
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
base: develop
Are you sure you want to change the base?
Conversation
Code Coverage (Ubuntu)DetailsDiff against developResults for commit: 81a6f47 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage (Windows)DetailsDiff against developResults for commit: 81a6f47 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
jlahovnik
left a comment
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.
Please add a test case and update the PR description!
f71a118 to
774456a
Compare
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.
on S1_SAR_GRD from earth_search, manifest.safe gets renamed to manifest.s when it shouldn't
| self.product.driver.guess_asset_key_and_roles( | ||
| "s3://foo/1/28/0/preview.png?collection=foo&items=foo", self.product | ||
| ), | ||
| ("previ.png", ["overview"]), |
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.
previ.png is not the expected output. Please update driver patterns of chars that could be included in other words (ew, iw, sm, raw) with something like r"(?<![A-Za-z])ew(?![A-Za-z])", to avoid letters on both sides
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.
these fixes should also be applied to other drivers
This PR fixes asset naming issues for Sentinel-1 assets coming from Planetary Computer and creodias_s3.
Asset names are now normalized to the name.extension format.
For example:
previ.png?foo → previ.png
calibration-vh-12a34b.xml → calibration-vh.xml
This PR updates the ASSET_KEYS_PATTERNS_ROLES and REPLACE_PATTERNS lists in sentinel1.py and adds new tests.