support UTM coordinate while subsetting in latitude/longitude#1464
Open
support UTM coordinate while subsetting in latitude/longitude#1464
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts latitude/longitude-based subsetting to pass both coordinates together into the UTM conversion, fixing a crash when only latitude was provided for UTM datasets and ensuring consistent y/x computation from geo bounds. Sequence diagram for updated UTM subsetting using lat/lon in subset_input_dict2boxsequenceDiagram
actor User
participant view_cli as view_cli
participant View as View_object
participant Subset as subset_module
participant Ut as ut_module
participant Coord as coord_object
User->>view_cli: run view.py with subset_lat and subset_lon
view_cli->>View: main(args)
View->>View: configure(inps)
View->>Subset: subset_input_dict2box(inps_dict, metadata)
Subset->>Ut: coordinate(metadata)
Ut-->>Subset: coord_object
Note over Subset,Coord: Compute sub_y from lat/lon
Subset->>Coord: lalo2yx(subset_lat, subset_lon)
Coord-->>Subset: y_from_latlon, x_from_latlon
Subset->>Subset: sub_y = y_from_latlon
alt subset_lon provided
Note over Subset,Coord: Compute sub_x from same lat/lon
Subset->>Coord: lalo2yx(subset_lat, subset_lon)
Coord-->>Subset: y_from_latlon2, x_from_latlon2
Subset->>Subset: sub_x = x_from_latlon2
else subset_x provided
Subset->>Subset: sub_x = subset_x
else no geo/pixel bounds
Subset->>Subset: sub_x = [0, width]
end
Subset-->>View: pix_box, geo_box
View-->>view_cli: ready_to_display_subset
view_cli-->>User: display subset velocity map
Flow diagram for updated subset_input_dict2box lat/lon to y/x logicflowchart TD
A["Start subset_input_dict2box"] --> B{Has subset_lat?}
B -- Yes --> C["coord = ut.coordinate(meta_dict)"]
C --> D["(sub_y, _) = coord.lalo2yx(subset_lat, subset_lon)"]
B -- No --> E{Has subset_y?}
E -- Yes --> F["sub_y = subset_y"]
E -- No --> G["sub_y = [0, length]"]
D --> H{Has subset_lon?}
F --> H
G --> H
H -- Yes --> I["(_, sub_x) = coord.lalo2yx(subset_lat, subset_lon)"]
H -- No --> J{Has subset_x?}
J -- Yes --> K["sub_x = subset_x"]
J -- No --> L["sub_x = [0, width]"]
I --> M["Return pix_box, geo_box with sub_y and sub_x"]
K --> M
L --> M
M --> N["End"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new calls to
coord.lalo2yx(subset_dict['subset_lat'], subset_dict['subset_lon'])assumesubset_lonis always present whensubset_latis provided; consider guarding against missing/Nonesubset_lonor falling back to the old behavior to avoid reintroducing theNoneTypeerror for lat-only subsetting. - Since both
sub_yandsub_xnow uselalo2yxwith the same inputs when lat/lon are provided, you could computey, x = coord.lalo2yx(...)once and reuse the results to avoid duplicated work and ensure consistent indexing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new calls to `coord.lalo2yx(subset_dict['subset_lat'], subset_dict['subset_lon'])` assume `subset_lon` is always present when `subset_lat` is provided; consider guarding against missing/`None` `subset_lon` or falling back to the old behavior to avoid reintroducing the `NoneType` error for lat-only subsetting.
- Since both `sub_y` and `sub_x` now use `lalo2yx` with the same inputs when lat/lon are provided, you could compute `y, x = coord.lalo2yx(...)` once and reuse the results to avoid duplicated work and ensure consistent indexing.
## Individual Comments
### Comment 1
<location> `src/mintpy/subset.py:195-196` </location>
<code_context>
if subset_dict.get('subset_lon', None):
- sub_x = coord.lalo2yx(None, subset_dict['subset_lon'])[1]
+ sub_x = coord.lalo2yx(subset_dict['subset_lat'],
+ subset_dict['subset_lon'])[1]
elif subset_dict['subset_x']:
sub_x = subset_dict['subset_x']
</code_context>
<issue_to_address>
**issue:** Accessing subset_dict['subset_lat'] here can raise a KeyError when only subset_lon is provided.
This branch only checks `subset_lon` via `subset_dict.get('subset_lon')`, but then unconditionally indexes `subset_dict['subset_lat']`. If only longitude is provided (no `subset_lat` key), this will now raise a `KeyError` instead of passing `None` as before. To keep the previous behavior while supporting combined lat/lon, use `subset_dict.get('subset_lat')` here or only pass latitude when its key is present.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
For files in WGS84, subset works using --lat or --lon alone, or using --lat/lon together; while for files in UTM, subset works ONLY when using --lat/lon together.
yunjunz
changed the title
yunjunz
changed the title
yunjunz
approved these changes
Feb 25, 2026
Member
yunjunz
left a comment
yunjunz
left a comment
There was a problem hiding this comment.
Adding a few lines to be compatible with the old behavior, when subsetting using only --lat or --lon also works for files in WGS84 coordinates.
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
2 participants
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
Encountered the following bug when trying to view a subset of a file that is in UTM coordinates. See print output below:
This bug was fixed by through this proposed patch. I can visualize the subset as expected now:
Reminders
Summary by Sourcery
Bug Fixes: