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

regression: projections need to be a crs #4306

Open
josch opened this issue Apr 10, 2022 · 7 comments
Open

regression: projections need to be a crs #4306

josch opened this issue Apr 10, 2022 · 7 comments

Comments

@josch
Copy link
Contributor

josch commented Apr 10, 2022

Hi,

thanks a lot for implementing proj7 support in 88241b3. Unfortunately, that commit introduces a call to proj_create_crs_to_crs which means that projections must now be a crs. I was using this pipeline before:

+proj=pipeline +step +proj=webmerc +step +proj=tinshift +file=/tmp/tinshift.json

and that doesn't work anymore as proj_create_crs_to_crs will fail with

proj_create_operations: source_crs is not a CRS

(It took me a while to figure out where the error comes from, so I also filed #4305 to improve error reporting)

Is this change intentional and do projections need to be a crs from now on? Projections that were not a crs worked fine in the past.

@artemp
Copy link
Member

artemp commented Apr 11, 2022

@josch 👍 thanks for highlighting^! According to proj docs cs2cs is using proj_create_crs_to_crs also. Is it possible to modify pipeline init string you mentioned above to work with cs2cs (proj9) ?

@josch
Copy link
Contributor Author

josch commented Apr 11, 2022

According to OSGeo/PROJ#3161 (comment) that string cannot be turned into a crs, so no.

@josch
Copy link
Contributor Author

josch commented Apr 13, 2022

Hi @artemp, I wanted to ask what your plans are. Do you plan to add another codepath that also supports projections that are not a crs? Or do you plan to drop support for non-crs projections with the next release? Thanks!

@mathisloge
Copy link
Collaborator

Just to note: with #4305 minimum proj version is bumpt up to 8.0.0 from 7.2.0

proj_context_errno_string was added with proj 8.0.0 https://proj.org/development/reference/functions.html#c.proj_context_errno_string

@artemp this was detected with the docker image PR #4267 since it is using ubuntu 21.04. The new ubuntu LTS 22.04 has currently 8.2.1 (https://packages.ubuntu.com/search?suite=default&section=all&arch=any&keywords=libproj-dev&searchon=names)

@artemp
Copy link
Member

artemp commented Apr 14, 2022

Just to note: with #4305 minimum proj version is bumpt up to 8.0.0 from 7.2.0

proj_context_errno_string was added with proj 8.0.0 https://proj.org/development/reference/functions.html#c.proj_context_errno_string

@artemp this was detected with the docker image PR #4267 since it is using ubuntu 21.04. The new ubuntu LTS 22.04 has currently 8.2.1 (https://packages.ubuntu.com/search?suite=default&section=all&arch=any&keywords=libproj-dev&searchon=names)

@mathisloge I think we should use proj_context_errno_string conditionally based on PROJ_VERSION

@artemp
Copy link
Member

artemp commented Apr 14, 2022

Hi @artemp, I wanted to ask what your plans are. Do you plan to add another codepath that also supports projections that are not a crs? Or do you plan to drop support for non-crs projections with the next release?

@josch I'm trying to figure out if this is something we can support. It would be super useful to see c++ code using proj_create to achieve this non-crs pipeline Do you have any pointers?

@josch
Copy link
Contributor Author

josch commented Apr 14, 2022

@mathisloge I think we should use proj_context_errno_string conditionally based on PROJ_VERSION

I filed #4307 to fix this.

@josch I'm trying to figure out if this is something we can support. It would be super useful to see c++ code using proj_create to achieve this non-crs pipeline Do you have any pointers?

I'm afraid I'm a complete proj noob. That's why I asked how to do it in OSGeo/PROJ#3161 (comment) but as the issue is closed I don't expect a reply.

I also asked the more general question on stackoverflow https://stackoverflow.com/questions/71823105 because all code I've seen so far that migrates to more recent proj versions just uses proj_create_crs_to_crs and calls it a day.

Since this is breaking my use-case for mapnik I'm willing to submit a pull request with an alternative code-path for non-crs input projections but first I have to learn how to do it in the first place. :(

josch added a commit to josch/mapnik that referenced this issue Apr 15, 2022
josch added a commit to josch/mapnik that referenced this issue Apr 15, 2022
josch added a commit to josch/mapnik that referenced this issue Apr 18, 2022
josch added a commit to josch/mapnik that referenced this issue Apr 18, 2022
josch added a commit to josch/mapnik that referenced this issue Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants