Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail consistently on network error and grid transformations #4302

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Oct 31, 2024

Currently when we need to use a remote grid that can't be opened, we return HUGE_VAL coordinates values and a proj_errno = PROJ_ERR_OTHER_NETWORK_ERROR, (I guess) as expected... But if we do following proj_trans() calls on the same transformation object, the grid transformation is ignored and we fallback to other methods (Helmert, ballpark, etc.). Fix that by consistently returning the same error values as the initial failed call.

Fixes pyproj4/pyproj#705

@snowman2 @jjimenezshaw @kbevers Thoughts? I guess this wins on consistency. The potential downside is that if you transform a batch of one million points using the same grid and the network timeout is 1second, you'll spend 1 million seconds, as the current implementation (as modified by this PR) retries to open the grid each time a point is transformed if it didn't manage to open it previously

Currently when we need to use a remote grid that can't be opened, we
return HUGE_VAL coordinates values and a proj_errno = PROJ_ERR_OTHER_NETWORK_ERROR,
(I guess) as expected... But if we do following proj_trans() calls on the same
transformation object, the grid transformation is ignored and we
fallback to other methods (Helmert, ballpark, etc.).
Fix that by consistently returning the same error values as the initial
failed call.

Fixes pyproj4/pyproj#705
@rouault rouault added the funded through GSP Work funded through the GDAL Sponsorship Program label Oct 31, 2024
@rouault
Copy link
Member Author

rouault commented Oct 31, 2024

Or maybe we should remember, for a given PJ* object, that we previously failed to open the grid, and return the same error as the initial failed attempt, without actually retrying ?

@kbevers
Copy link
Member

kbevers commented Nov 1, 2024

Or maybe we should remember, for a given PJ* object, that we previously failed to open the grid, and return the same error as the initial failed attempt, without actually retrying ?

I think this is the better option. If one or more coordinates in a dataset fails [0] to transform the dataset is corrupted, so we may as well do our best to make sure that it is clear to the user. You're more likely to notice that something is wrong when all or most of your coordinates are HUGE_VAL's compared to only a few.

[0] Due to network issues, assuming it would transform ok otherwise

@rouault
Copy link
Member Author

rouault commented Nov 1, 2024

I think this is the better option.

implemented

Copy link
Contributor

@snowman2 snowman2 left a comment

Choose a reason for hiding this comment

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

Thanks @rouault 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 9.5 Backport to 9.5 branch funded through GSP Work funded through the GDAL Sponsorship Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PROJ_NETWORK: First transform fails, succeeds afterwards
3 participants