tsview: support --shp-file options as well#1436
Merged
yunjunz merged 2 commits intoinsarlab:mainfrom Nov 22, 2025
Merged
Conversation
+ support --shp-file related options in tsview.py as well + add `is_native_reference_point()` for easier re-use and allow for the potential geo2radar coordinate conversion error, which is usually less than or about 1 pixel.
Contributor
Reviewer's GuideAdds shapefile plotting support to tsview, refactors reference-point handling into a reusable helper that tolerates small geo2radar errors, and slightly improves CLI/help text and logging around reference point display. Sequence diagram for tsview reference point handling with geo2radar tolerancesequenceDiagram
actor User as "User"
participant CLI as "tsview CLI entrypoint"
participant Parser as "Argument parser"
participant Core as "mintpy.tsview"
participant Init as "read_init_info()"
participant Timeseries as "read_timeseries_data()"
participant Helper as "is_native_reference_point()"
User->>CLI: "Run tsview with optional ref_yx and subset"
CLI->>Parser: "Parse CLI arguments (including ref_yx, pix_box, multilook_num)"
Parser-->>CLI: "Return populated inps object"
CLI->>Init: "read_init_info(inps)"
Init->>Helper: "is_native_reference_point(inps.ref_yx, atr)"
Helper-->>Init: "True if ref_yx within tolerance of native REF_Y/X"
Init->>Init: "Check subset box against ref_yx when native and within coverage"
Init->>Init: "If native ref_yx outside subset, set inps.disp_ref_pixel = False and log warning"
Init-->>CLI: "Updated inps with display-ref flags"
CLI->>Timeseries: "read_timeseries_data(inps)"
Timeseries->>Helper: "is_native_reference_point(inps.ref_yx, atr)"
Helper-->>Timeseries: "False if ref_yx not within tolerance of native REF_Y/X"
Timeseries->>Timeseries: "If non-native ref_yx, subset_and_multilook_yx(...) and re-reference data"
Timeseries-->>CLI: "Timeseries data referenced according to ref_yx behavior"
Updated class diagram for tsview reference point and CLI shape argument utilitiesclassDiagram
class MintpyTsviewCore {
+read_init_info(inps)
+read_timeseries_data(inps)
+subset_and_multilook_yx(yx, pix_box=None, multilook_num=1)
+is_native_reference_point(ref_yx, atr, max_err=2)
}
class ArgUtils {
+add_shape_argument(parser)
+add_save_argument(parser)
+add_subset_argument(parser)
+add_reference_argument(parser)
+add_map_argument(parser)
+add_memory_argument(parser)
}
class TsviewCLI {
+create_parser(subparsers=None)
}
TsviewCLI ..> ArgUtils : "uses add_shape_argument() to add --shp-* options"
TsviewCLI ..> ArgUtils : "uses other add_*_argument() utilities"
MintpyTsviewCore ..> MintpyTsviewCore : "read_init_info() and read_timeseries_data() call is_native_reference_point()"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In is_native_reference_point(), you only guard against missing 'REF_Y' but access 'REF_X' unconditionally, which can raise a KeyError for datasets without REF_X; consider checking for 'REF_X' as well or using atr.get with a sensible default.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In is_native_reference_point(), you only guard against missing 'REF_Y' but access 'REF_X' unconditionally, which can raise a KeyError for datasets without REF_X; consider checking for 'REF_X' as well or using atr.get with a sensible default.
## Individual Comments
### Comment 1
<location> `src/mintpy/tsview.py:271-274` </location>
<code_context>
+ max_err - int, maximum allowable error to account for
+ potential geo2radar coordinate conversion error
+ """
+ if 'REF_Y' not in atr.keys():
+ return False
+
+ ref_x = int(atr['REF_X'])
+ ref_y = int(atr['REF_Y'])
+ x0, x1 = ref_x - max_err, ref_x + max_err
</code_context>
<issue_to_address>
**issue:** Guard against missing REF_X key as well as REF_Y when checking native reference point.
This only checks for 'REF_Y' but then indexes atr['REF_X'], which will raise a KeyError if 'REF_X' is missing or misnamed. Please either check for both keys before use (e.g., `if 'REF_X' not in atr or 'REF_Y' not in atr: return False`) or use `atr.get` with a defined fallback to avoid crashes when metadata is incomplete or varies between datasets.
</issue_to_address>
### Comment 2
<location> `src/mintpy/tsview.py:279-282` </location>
<code_context>
def is_native_reference_point(ref_yx, atr, max_err=2):
"""Check if the given ref_yx is the native reference point or not.
Parameters: ref_yx - list of int, input reference point in row/col
atr - dict, attributes, to retrieve the native REF_Y/X
max_err - int, maximum allowable error to account for
potential geo2radar coordinate conversion error
"""
if 'REF_Y' not in atr.keys():
return False
ref_x = int(atr['REF_X'])
ref_y = int(atr['REF_Y'])
x0, x1 = ref_x - max_err, ref_x + max_err
y0, y1 = ref_y - max_err, ref_y + max_err
if (x0 <= ref_yx[1] < x1 and y0 <= ref_yx[0] < y1):
return True
else:
return False
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
- Simplify boolean if expression ([`boolean-if-exp-identity`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/boolean-if-exp-identity/))
- Remove unnecessary casts to int, str, float or bool ([`remove-unnecessary-cast`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-cast/))
```suggestion
return x0 <= ref_yx[1] < x1 and y0 <= ref_yx[0] < y1
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
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
support --shp-file related options in tsview.py as well
add
is_native_reference_point()for easier re-use and allow for the potential geo2radar coordinate conversion error, which is usually less than or about 1 pixel.Reminders
Summary by Sourcery
Extend tsview functionality to better handle reference points and to support shape-file plotting options via the CLI.
New Features:
Enhancements: