Skip to content

load_data: narrow bare except and report the prep_* failure cause - #1517

Merged
yunjunz merged 1 commit into
insarlab:mainfrom
s-sasaki-earthsea-wizard:narrow_prep_except
Aug 24, 2026
Merged

load_data: narrow bare except and report the prep_* failure cause#1517
yunjunz merged 1 commit into
insarlab:mainfrom
s-sasaki-earthsea-wizard:narrow_prep_except

Conversation

@s-sasaki-earthsea-wizard

@s-sasaki-earthsea-wizard s-sasaki-earthsea-wizard commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Description of proposed changes

Fixes #1516.

Thanks @yunjunz for the quick feedback on the issue — as discussed there, this preserves the existing fallback for ordinary prep_* failures: prepare_metadata() still warns and continues, so workflows that rely on a pre-existing inputs/ directory keep working. What changes is what the warning reports, and that control-flow exceptions are no longer swallowed (see below).

The three bare except: clauses around the prep_nisar / prep_isce / prep_gmtsar calls are narrowed to except Exception as e, and the warning now includes the exception type and message. The hardcoded module names in the messages are replaced with the script_name variable already in scope.

Before (a valid HDF5 container missing the required GUNW datasets, MintPy 1.6.4):

UserWarning: prep_nisar.py failed. Assuming its result exists and continue...

After (same input):

UserWarning: prep_nisar.py failed (ValueError: NISAR auto (frequencyA) data
for polarization 'HH' was not found in ./gunw_20260101_20260113.h5. Missing
path: /science/LSAR/GUNW/grids/frequencyA/unwrappedInterferogram. Use
--frequency B for frequencyB products.). Assuming its result exists and
continue...

For ordinary exceptions the exit status stays 0 and the run continues as before.
Narrowing the catch also fixes the second problem noted in #1516: control-flow exceptions such as KeyboardInterrupt / SystemExit are no longer swallowed, so Ctrl-C during a long prep_* run aborts as expected.

Verified locally on all three affected branches (nisar / isce / gmtsar) with minimal reproductions — no real data needed — plus pre-commit run --all-files (13 hooks passed, 1 skipped). In each case the warning now carries the exception type and message, and the exit status stays 0:

nisar — reproduction from #1516 (valid HDF5 container missing the required GUNW datasets)
mkdir repro_nisar && cd repro_nisar

python -c "
import h5py
with h5py.File('gunw_20260101_20260113.h5', 'w') as f:
    f.create_group('science/LSAR/GUNW')
"
touch dem.vrt
python -c "
import mintpy, os, shutil
shutil.copy(os.path.join(os.path.dirname(mintpy.__file__), 'defaults/smallbaselineApp.cfg'), '.')
"

cat > repro.txt <<'EOF'
mintpy.load.processor = nisar
mintpy.load.unwFile   = ./gunw_*.h5
mintpy.load.demFile   = ./dem.vrt
EOF

load_data.py -t smallbaselineApp.cfg repro.txt; echo "exit=$?"

Produces the "After" warning shown above; exit=0.

isce — nonexistent metadata file (no dummy files needed)

The isce branch falls back to -m auto when the metadata file is not found,
so pointing the template at nonexistent paths is enough:

mkdir repro_isce && cd repro_isce

python -c "
import mintpy, os, shutil
shutil.copy(os.path.join(os.path.dirname(mintpy.__file__), 'defaults/smallbaselineApp.cfg'), '.')
"

cat > repro.txt <<'EOF'
mintpy.load.processor = isce
mintpy.load.metaFile  = ./reference/IW1.xml
mintpy.load.demFile   = ./merged/geom_reference/hgt.rdr
mintpy.load.unwFile   = ./merged/interferograms/*/filt_fine.unw
EOF

load_data.py -t smallbaselineApp.cfg repro.txt; echo "exit=$?"
UserWarning: prep_isce.py failed (ValueError: Un-recognized ISCE processor
for metadata file: auto). Assuming its result exists and continue...

exit=0.

gmtsar — minimal custom template missing required keys
mkdir repro_gmtsar && cd repro_gmtsar

python -c "
import mintpy, os, shutil
shutil.copy(os.path.join(os.path.dirname(mintpy.__file__), 'defaults/smallbaselineApp.cfg'), '.')
"

cat > repro.txt <<'EOF'
mintpy.load.processor = gmtsar
mintpy.load.unwFile   = ./merged/*/unwrap_ll.grd
EOF

load_data.py -t smallbaselineApp.cfg repro.txt; echo "exit=$?"
UserWarning: prep_gmtsar.py failed (KeyError: 'mintpy.load.metaFile').
Assuming its result exists and continue...

exit=0.

Note: #1510 adds an isce3 branch to the same function; I am happy to rebase this branch around it in whichever order is more convenient for #1510.


Disclosure: this change was developed with AI assistance. The change was reviewed and verified by the author.

Reminders

  • Fix load_data: bare except hides prep_*.py failures while the run reports success #1516
  • Pass Pre-commit check (green) — passes locally (pre-commit run --all-files)
  • 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. — behavior changes (warning content; control-flow exceptions now propagate) are described above.
  • If adding new functionality, add a detailed description to the documentation and/or an example. — n/a, no new functionality.

Summary by Sourcery

Improve metadata preparation failure reporting while preserving fallback behavior for recoverable preparation errors.

Bug Fixes:

  • Report the exception type and message when metadata preparation fails for NISAR, ISCE, or GMTSAR workflows.
  • Allow control-flow exceptions such as KeyboardInterrupt and SystemExit to propagate instead of being swallowed during metadata preparation.

Enhancements:

  • Use the active preparation script name in failure warnings while preserving the existing fallback behavior for ordinary exceptions.

@sourcery-ai

sourcery-ai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Narrow the bare exception handling around prep_* metadata preparation calls in load_data.prepare_metadata and enrich the warning messages with the specific exception type, message, and dynamic script name, while preserving the existing fallback behavior and allowing control-flow exceptions to propagate.

Sequence diagram for metadata preparation exception handling

sequenceDiagram
    participant Caller
    participant prepare_metadata
    participant prep_module
    participant Warnings

    Caller->>prepare_metadata: prepare_metadata(iDict)
    prepare_metadata->>prep_module: main(iargs)
    alt ordinary Exception
        prep_module-->>prepare_metadata: Exception
        prepare_metadata->>Warnings: warn(script_name, exception type, message)
        prepare_metadata-->>Caller: continue with fallback
    else control-flow exception
        prep_module-->>prepare_metadata: KeyboardInterrupt or SystemExit
        prepare_metadata-->>Caller: propagate exception
    else success
        prep_module-->>prepare_metadata: return
        prepare_metadata-->>Caller: continue normally
    end
Loading

File-Level Changes

Change Details Files
Improve exception handling and warning messages around prep_* metadata preparation calls while preserving fallback behavior.
  • Replace three bare except: blocks wrapping prep_module.main(iargs) with except Exception as e to avoid swallowing control-flow exceptions like KeyboardInterrupt and SystemExit.
  • Update warning text to include the dynamic script_name instead of hardcoded module names and to append the caught exception type and message via an f-string.
  • Keep the workflow behavior unchanged: on ordinary exceptions, emit a UserWarning and continue execution under the assumption that prep_* outputs already exist.
src/mintpy/load_data.py

Assessment against linked issues

Issue Objective Addressed Explanation
#1516 Narrow the exception handling around the nisar, isce, and gmtsar prep_* calls so that ordinary exceptions are still handled with the existing continue-on-failure behavior, while KeyboardInterrupt and SystemExit propagate normally.
#1516 Report the actual prep_* failure cause, including the exception type and message, in the warning emitted by prepare_metadata().
#1516 Use the in-scope script_name rather than hardcoded prep module names in the failure warnings for all three affected branches.

Possibly linked issues


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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


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.

@yunjunz yunjunz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, thank you @s-sasaki-earthsea-wizard for the PR!

@yunjunz
yunjunz merged commit 9ede611 into insarlab:main Aug 24, 2026
9 checks passed
@welcome

welcome Bot commented Aug 24, 2026

Copy link
Copy Markdown

🎉 🎉 🎉 Congrats on merging your first pull request! We here at behaviorbot are proud of you! 🎉 🎉 🎉

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.

load_data: bare except hides prep_*.py failures while the run reports success

2 participants