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.

Sign up for GitHub

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

Jump to bottom

libgproj: Use SRID for PJ object creation when possible #2394

Merged
merged 4 commits into from
Jan 10, 2023
Merged

libgproj: Use SRID for PJ object creation when possible #2394

merged 4 commits into from
Jan 10, 2023

Conversation

HuidaeCho
Copy link
Member

This PR tries to address #2257 by using SRID, not GRASS key-value pairs, for creating an PJ object. Here is a related Q&A.

@HuidaeCho HuidaeCho linked an issue May 23, 2022 that may be closed by this pull request
[Bug] g.region -n Flag broken #2257
Closed
@HuidaeCho HuidaeCho mentioned this pull request May 23, 2022
Closed
@HuidaeCho HuidaeCho added bug Something isn't working C Related code is in C labels May 23, 2022
@HuidaeCho HuidaeCho changed the title Use SRID for PJ object creation when possible libgproj: Use SRID for PJ object creation when possible May 23, 2022
@petrasovaa petrasovaa requested a review from metzm May 23, 2022 12:48
@petrasovaa petrasovaa added this to the 8.4.0 milestone May 23, 2022
metzm
metzm requested changes May 27, 2022
View reviewed changes
Copy link
Contributor

@metzm metzm left a comment

Choose a reason for hiding this comment

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

The idea is correct, but this PR does not fix cases where no SRID is available and a bound CRS is provided. Should be easy to add.

lib/proj/get_proj.c Outdated Show resolved Hide resolved
@HuidaeCho
Copy link
Member Author

@metzm Do we ever hit this PROJ error when pj is already not NULL?

grass/lib/proj/get_proj.c

Lines 275 to 278 in 830f31d

int perr = proj_errno(pj);
if (perr)
G_fatal_error("PROJ 5 error %d", perr);

@HuidaeCho HuidaeCho requested a review from metzm May 30, 2022 18:17
@HuidaeCho
Copy link
Member Author

@metzm Do we ever hit this PROJ error when pj is already not NULL?

grass/lib/proj/get_proj.c

Lines 275 to 278 in 830f31d

int perr = proj_errno(pj);
if (perr)
G_fatal_error("PROJ 5 error %d", perr);

I think we can remove this check.

https://proj.org/development/reference/functions.html#c.proj_create_argv

If creation of the transformation object fails, the function returns 0 and the PROJ error number is updated.

@metzm
Copy link
Contributor

metzm commented Jul 1, 2022

Apparently proj_errno(pj) can still return an error code, if pj is not NULL. At least this is used in various places in PROJ itself.

metzm
metzm approved these changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@metzm metzm left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! This will not be the only one regarding the evolving PROJ lib I guess...

@HuidaeCho HuidaeCho merged commit 76d27ff into OSGeo:main Jan 10, 2023
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@HuidaeCho @ninsbl
libgproj: Use SRID for PJ object creation when possible (OSGeo#2394)
e8ddba0 
* Use SRID for PJ creation

* Use proj_get_source_crs()

* Tabs to spaces
@HuidaeCho HuidaeCho deleted the pj_use_srid branch June 5, 2023 03:40
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@HuidaeCho @neteler
libgproj: Use SRID for PJ object creation when possible (OSGeo#2394)
3a10e22 
* Use SRID for PJ creation

* Use proj_get_source_crs()

* Tabs to spaces
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metzm metzm metzm approved these changes

Assignees
No one assigned
Labels
bug Something isn't working C Related code is in C
Projects
None yet
Milestone
8.3.0
Development

Successfully merging this pull request may close these issues.

[Bug] g.region -n Flag broken
3 participants
@HuidaeCho @metzm @petrasovaa