load_data: narrow bare except and report the prep_* failure cause - #1517
Merged
yunjunz merged 1 commit intoAug 24, 2026
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideNarrow 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 handlingsequenceDiagram
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
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
yunjunz
approved these changes
Aug 24, 2026
yunjunz
left a comment
Member
There was a problem hiding this comment.
Looks good to me, thank you @s-sasaki-earthsea-wizard for the PR!
|
🎉 🎉 🎉 Congrats on merging your first pull request! We here at behaviorbot are proud of you! 🎉 🎉 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-existinginputs/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 theprep_nisar/prep_isce/prep_gmtsarcalls are narrowed toexcept Exception as e, and the warning now includes the exception type and message. The hardcoded module names in the messages are replaced with thescript_namevariable already in scope.Before (a valid HDF5 container missing the required GUNW datasets, MintPy 1.6.4):
After (same input):
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/SystemExitare no longer swallowed, so Ctrl-C during a longprep_*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)
Produces the "After" warning shown above;
exit=0.isce — nonexistent metadata file (no dummy files needed)
The isce branch falls back to
-m autowhen the metadata file is not found,so pointing the template at nonexistent paths is enough:
exit=0.gmtsar — minimal custom template missing required keys
exit=0.Note: #1510 adds an
isce3branch 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
pre-commit run --all-files)Summary by Sourcery
Improve metadata preparation failure reporting while preserving fallback behavior for recoverable preparation errors.
Bug Fixes:
Enhancements: