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

Add sentinel1.grd subpackage #11

Merged
merged 9 commits into from
Sep 3, 2021
Merged

Add sentinel1.grd subpackage #11

merged 9 commits into from
Sep 3, 2021

Conversation

lossyrob
Copy link
Member

@lossyrob lossyrob commented Sep 3, 2021

Related Issue(s):
stactools-packages/sentinel1-grd#2

Description:
This PR moves the code for processing Sentinel1 GRD that currently exists in https://github.com/stactools-packages/sentinel1-grd into a subpackage of this project.

The project is refactored to include two subpackages - rtc and grd. The existing code is moved into the rtc subpackage and the code from sentinel1-grd is moved to the grd subpackage. The top-level stac sentinel1 command is now a group that has rtc and grd as subcommands, which behave how the previous stac sentinel1 and stac sentinel1grd commands worked, respectively.

I've updated the README to account for these two datasets along with the examples and tests.

NOTE: The GRD code was created by @maximlamare, all credit to them - I'm just moving it over. @maximlamare if you'd like to rebase these commits and force push to this branch so that your account is associate with the commits by all means do so!

PR checklist:

  • [*] Code is formatted (run scripts/format).
  • [*] Code lints properly (run scripts/lint).
  • [*] Tests pass (run scripts/test).
  • [*] Documentation has been updated to reflect changes, if applicable.
  • [*] Changes are added to the CHANGELOG.

This seems to have changed in a recent release.

Also pin to a minor version, although it seems like 2.3.* has been out
for a while so they are potentially not following semver, and this may
not have avoided the current issue.
Also avoid using un-imported subpackages of pystac by accessing core
types directly from the top level package.
Also add the exampel item.json from sentinel1-grd
@lossyrob
Copy link
Member Author

lossyrob commented Sep 3, 2021

@maximlamare invited you to this repository; I can't assign you as a reviewer until that invite is accepted, but would like you to please review!

Copy link
Collaborator

@maximlamare maximlamare left a comment

Choose a reason for hiding this comment

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

There were a few details that I forgot to change in the setup.cfg, but the merge has taken care of them. Looks all good to me. I appreciate the help I received with this.

@scottyhq
Copy link
Collaborator

scottyhq commented Sep 3, 2021

thanks @lossyrob! I'm going to try a release of the current code on pypi since we never did that, then we can add this PR to the CHANGELOG and make a new release

Comment on lines +21 to +23
# Fixed properties
sar_ext.frequency_band = FrequencyBand("C")
sar_ext.center_frequency = 5.405
Copy link
Collaborator

Choose a reason for hiding this comment

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

in the future would be good to consolidate common properties for GRD,RTC,SLC, etc under sentinel1/properties.py

Comment on lines +23 to +27
root = XmlElement.from_file(self.href)
data_object_section = root.find("dataObjectSection")
if data_object_section is None:
raise ManifestError(
f"Manifest at {self.href} does not have a dataObjectSection")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@maximlamare just wanted to point out https://github.com/bopen/xarray-sentinel/blob/main/xarray_sentinel/esa_safe.py , which seems to be a nice approach for parsing SAFE files using the accompanying .xsd files for XML validation and parsing

Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we have contributed all sensors we had in mind, I could take a look and propose an update to account for this.

@scottyhq scottyhq merged commit 6de2414 into main Sep 3, 2021
@lossyrob lossyrob deleted the feature/rde/add-grd branch September 3, 2021 17:49
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.

3 participants