Skip to content

Conversation

KavyaKinjalk
Copy link

@KavyaKinjalk KavyaKinjalk commented May 5, 2025

Description of proposed changes

adds compatibility for HyP3's INSAR_ISCE_MULTI_BURST job format.

Here as well, HyP3 generates a single interferogram from multiple reference and secondary bursts. To maintain compatibility with MintPy's data model, the first reference granule is selected to represent the acquisition metadata.

With this, MintPy can process HyP3 multi-burst ISCE2 products for time-series analysis and SBAS workflows.

Reminders

  • Fix #xxxx
  • Pass Pre-commit check (green)
  • Pass Codacy code review (green)
  • Pass Circle CI test (green)
  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • If modifying functionality, describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

Copy link

welcome bot commented May 5, 2025

💖 Thanks for opening this pull request! Please check out our contributing guidelines. 💖
Keep in mind that all new features should be documented. It helps to write the comments next to the code or below your functions describing all arguments, and return types before writing the code. This will help you think about your code design and usually results in better code.

Copy link
Contributor

sourcery-ai bot commented May 5, 2025

Reviewer's Guide

This pull request adds support for the Hyp3 INSAR_ISCE_MULTI_BURST product format by modifying the add_hyp3_metadata function. It introduces logic to detect this format based on the job ID derived from the directory name structure and updates metadata parsing to handle potentially multiple reference granules, selecting only the first one. Existing metadata extraction logic is extended to accommodate this new format.

File-Level Changes

Change Details Files
Detect and parse metadata for the isce2_multi_burst format.
  • Added conditional logic to identify isce2_multi_burst based on the job ID parsed from the directory name.
  • Extracted date information specifically for the isce2_multi_burst naming convention.
src/mintpy/prep_hyp3.py
Adapt metadata handling and documentation for the multi-burst format.
  • Updated ReferenceGranule parsing to select only the first granule when multiple are listed.
  • Extended existing metadata field population logic (e.g., relative_orbit) to apply to the isce2_multi_burst type.
  • Updated the function docstring to describe the new format and its metadata structure.
src/mintpy/prep_hyp3.py

Possibly linked issues

  • #0: The PR implements the HyP3 multi-burst support that the issue requests to be included in a new release.
  • regularly updated docker image #123: PR adds support for INSAR_ISCE_MULTI_BURST HyP3 format, contributing to issue goal.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @KavyaKinjalk - I've reviewed your changes - here's some feedback:

  • Using hardcoded indices for filename parsing reduces maintainability; consider alternatives like named constants or regular expressions.
  • Consider refactoring the duplicated relative orbit calculation into a helper function.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Alex-Lewandowski
Copy link
Contributor

Alex-Lewandowski commented May 6, 2025

Hi @KavyaKinjalk,

The multi-burst InSAR HyP3 product is still very new and will likely have an update to its naming convention in the near future.

I am using the following as a temporary workaround to rename the files so they can be loaded with prep_hyp3.py. It's messy but gets the job done while things are in a transient state.

from pathlib import Path

data_path = Path("path/to/your/multi-burst/hyp3/stack")

all_paths = sorted(list(data_path.glob('*/*.t*')))
for pth in all_paths:
    parts = pth.name.split("_")
    new_name = Path(f"{'_'.join(parts[:2])}_IWMULTIBURST_{parts[10]}_{parts[11]}_{'-'.join(parts[2:10])}_{'-'.join(parts[12:14])}_{'_'.join(parts[14:])}")
    pth.rename(pth.parent/new_name)

@KavyaKinjalk
Copy link
Author

KavyaKinjalk commented May 7, 2025 via email

@forrestfwilliams
Copy link
Collaborator

Hey @KavyaKinjalk just wanted to let you know that we're working on this at ASF and have started a GitHub issue to decide on a new name.

@forrestfwilliams
Copy link
Collaborator

Hey guys, sorry this has taken so long but we're finally getting around to updating the naming convention of these products to be compatible with MintPy without requiring changes. This documentation PR describes the format we're moving to.

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