search

stactools-packages / sentinel1-grd

stactools package for Sentinel-1 Level-1 Ground Range Detected (GRD) products
Other
1 stars 0 forks source link

Merge with stactools-packages/sentinel1 #2

Open lossyrob opened 3 years ago

lossyrob commented 3 years ago

There's an existing sentinel1 stactools package which handles the AWS RTC dataset. It seems to have the same dependencies as this project, so merging the two wouldn't cause any dependency conflict. Is there a reason to keep these two codebases separate? Otherwise I think it would be wise to consolidate all of the sentinel 1 processing into single repository so that the code can build on each other's and users don't have to look to multiple packages for processing Sentinel 1 data.

maximlamare commented 3 years ago

Hi @lossyrob ,

That is something I considered, but didn't have the knowledge to merge my work into the existing . It definitely makes sense to merge the two together, particularly as I would like to add SLC next (having 3 packages wouldn't make sense).

I would appreciate any pointers that would help me understand how to integrate the GRD package into sentinel1, if that's not being a pain!

lossyrob commented 3 years ago

Sounds good! @scottyhq as the maintainer of the sentinel1 package, does this seem like a good plan to you?

My suggestion would be to create subpackages in the sentinel1 package named grd and rtc, and move the existing sentinel1 code to the rtc subpackage, and this code to the grd subpackage. It will take some top-level code to tie together the CLI into subcommands, but should be straightforward. Happy to help out with that code change.

scottyhq commented 3 years ago

My suggestion would be to create subpackages in the sentinel1 package named grd and rtc, and move the existing sentinel1 code to the rtc subpackage, and this code to the grd subpackage.

Great! Excited to see this effort @maximlamare and would love to have more Sentinel1 under the same repo. Eventually it would be nice to share common pieces of code, but for now subpackages is probably easiest.

I just looked at https://github.com/stactools-packages/sentinel2 thinking there might be an example of S2 L2A and S2 L1C under the same repo, but seems that's not yet the case. @matthewhanson also started an effort in the past to unify multiple sentinel datasets for AWS EarthSearch under the same repository, https://github.com/sat-utils/sat-stac-sentinel, but I believe the goal is to use stactools-packages going forward? The one other point I'll link to for now is the past discussion of sentinel1 RTC and GRD scene footprints https://github.com/stac-utils/stactools/pull/84#issuecomment-810697975.

Happy to help out with that code change.

That would be much appreciated!

maximlamare commented 3 years ago

Thanks for the info on Sentinel-1 sub-packages. As a side-note, we are almost finished with Sentinel-3, where we have built 1 package for OLCI and SLSTR.

For the sub-packages: maybe @lossyrob you can provide some quick indications of what to do? I will also benefit from some help of a developer in one of our teams, so once I know overall how the process is, I can work on the integration of GRD as a subpackage with his help! But it might be more efficient if you set the steps to be followed?

The one other point I'll link to for now is the past discussion of sentinel1 RTC and GRD scene footprints stac-utils/stactools#84 (comment).

I'll look to implement the function discussed in the GRD sub-package. Thanks for the link.

lossyrob commented 3 years ago

@maximlamare given my familiarity with the stactools packages layouts and an idea of how it should work, it might be quickest to take some time either today or tomorrow to do make a PR against the other project, which I can then tag you in for review. Sound good?

This would just be a code move and refactor, so if there's an update to the footprint based on the discussion above that could be layered on in the new repo, or added here first if you're quicker than I am!

lossyrob commented 3 years ago

This is done - we should decide what to do with this repo...we could mark it as deprecated or delete, what do you think?

maximlamare commented 3 years ago

The sentinel1_grd? I guess we can delete it, since it's redundant now!