`gnss`: handle longitude <=-180 or >=360 by yunjunz · Pull Request #1290 · insarlab/MintPy

yunjunz · 2024-11-11T09:45:11Z

Description of proposed changes

utils/utils0.py: add standardize_longitude() to handle the longitude data range
- ensure longitude is within (-180, 360)
- could specify the data range format, in either [0, 360) or (-180, 180] via lon_range arg
objects/gnss.py： call ut.standardize_longitude() in all the child class of GNSS

This error occurs for the GNSS_UNR sites in Japan, where the longitude of G073 can be -229. In the old code, this results in a NaN value for inc/az_angle extraction from the geometry file.

Reminders

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.

Summary by Sourcery

Ensure longitude values are correctly handled within the GNSS module by introducing a function to set longitude ranges, preventing errors in data processing.

Bug Fixes:

Fix longitude handling for GNSS_UNR sites to prevent NaN values by ensuring longitude is within the valid range.

Enhancements:

Introduce a new function _set_longitude_range() to standardize longitude values across GNSS data processing.

+ objects/gnss.py: add `_set_longitude_range()` to handle the longitude data range - ensure longitude is within (-180, 360) - could specify the data range format, in either [0, 360) or (-180, 180] via `lon_range` arg + call `_set_longitude_range()` in all the child class of GNSS This error is happending for the GNSS_UNR sites in Japan, where the longitude of G073 can be -229, resulting in NaN value for inc/az_angle extration from the geometry file, in the old code.

sourcery-ai · 2024-11-11T09:45:15Z

Reviewer's Guide by Sourcery

This PR introduces a new longitude handling mechanism in the GNSS module to properly handle longitude values that fall outside the standard ranges. The implementation adds a new utility function _set_longitude_range() that normalizes longitude values to either (-180, 180] or [0, 360) ranges, and updates all GNSS child classes to use this function.

Class diagram for GNSS longitude handling update

classDiagram
    class GNSS {
        +_set_longitude_range(lon, lon_range)
    }
    class GNSSChild1 {
        +get_site_lat_lon(print_msg)
    }
    class GNSSChild2 {
        +get_site_lat_lon(print_msg)
    }
    class GNSSChild3 {
        +get_site_lat_lon(print_msg)
    }
    GNSS <|-- GNSSChild1
    GNSS <|-- GNSSChild2
    GNSS <|-- GNSSChild3
    note for GNSS "New method _set_longitude_range added to handle longitude normalization"
    note for GNSSChild1 "Updated to use _set_longitude_range for longitude normalization"
    note for GNSSChild2 "Updated to use _set_longitude_range for longitude normalization"
    note for GNSSChild3 "Updated to use _set_longitude_range for longitude normalization"

File-Level Changes

Change	Details	Files
Added new longitude normalization utility function	Implements longitude range normalization to either (-180, 180] or [0, 360) Handles both scalar and numpy array inputs Ensures input values are first normalized to (-180, 360) range before final range conversion	`src/mintpy/objects/gnss.py`
Updated longitude handling across GNSS classes	Replaced direct longitude adjustments with calls to _set_longitude_range() Standardized longitude range to (-180, 180] across all GNSS child classes Added longitude normalization after loading site coordinates	`src/mintpy/objects/gnss.py`

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.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a 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. You can also use
this command to specify where the summary should be inserted.

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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

codeautopilot · 2024-11-11T09:45:22Z

PR summary

This Pull Request addresses a bug in the GNSS module related to handling longitude values that fall outside the typical range, specifically for GNSS_UNR sites in Japan. The changes introduce a new function, standardize_longitude(), which ensures longitude values are standardized within a specified range, either (-180, 180] or [0, 360). This prevents errors such as NaN values during data processing, particularly in the inc/az_angle extraction from geometry files.

Suggestion

Consider adding unit tests for the standardize_longitude() function to ensure its robustness and correctness across various input scenarios. This will help maintain code quality and prevent future regressions. Additionally, updating the documentation to reflect these changes and the new function's usage would be beneficial for users and developers.

Disclaimer: This comment was entirely generated using AI. Be aware that the information provided may be incorrect.

Current plan usage: 0.00%

Have feedback or need help?
Discord
Documentation
support@codeautopilot.com

sourcery-ai

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

Here's what I looked at during the review

🟡 General issues: 1 issue 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.}

src/mintpy/objects/gnss.py

+ rename `_set_longitude_range()` to `standardize_longitude()` + move the function from objects/gnss.py to utils/utils0.py

+ `utils.utils0.py`: add `standardize_longitude()` to handle the longitude data range - ensure longitude is within (-180, 360) - could specify the data range format, in either [0, 360) or (-180, 180] via `lon_range` arg + `objects.gnss.py`: call `ut.standardize_longitude()` in all the child class of GNSS This error occurs for the GNSS_UNR sites in Japan, where the longitude of G073 can be -229. In the old code, this results in a NaN value for inc/az_angle extraction from the geometry file.

sourcery-ai bot reviewed Nov 11, 2024

View reviewed changes

src/mintpy/objects/gnss.py Outdated Show resolved Hide resolved

src/mintpy/objects/gnss.py Outdated Show resolved Hide resolved

rename and move to utils0.py

be03e45

+ rename `_set_longitude_range()` to `standardize_longitude()` + move the function from objects/gnss.py to utils/utils0.py

yunjunz merged commit c7e77cc into insarlab:main Nov 11, 2024

yunjunz deleted the gnss branch November 11, 2024 10:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`gnss`: handle longitude <=-180 or >=360#1290

`gnss`: handle longitude <=-180 or >=360#1290
yunjunz merged 2 commits intoinsarlab:mainfrom
yunjunz:gnss

yunjunz commented Nov 11, 2024 •

edited

Loading

Uh oh!

sourcery-ai bot commented Nov 11, 2024 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

codeautopilot bot commented Nov 11, 2024 •

edited

Loading

Uh oh!

sourcery-ai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

yunjunz commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

Class diagram for GNSS longitude handling update

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

codeautopilot bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

Suggestion

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yunjunz commented Nov 11, 2024 •

edited

Loading

sourcery-ai bot commented Nov 11, 2024 •

edited

Loading

codeautopilot bot commented Nov 11, 2024 •

edited

Loading