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

Add proj_normalize_for_visualization() #1387

Merged
merged 1 commit into from Mar 29, 2019

Conversation

@rouault
Copy link
Member

@rouault rouault commented Mar 28, 2019

Fixes #1301

This function takes the output PJ from proj_create_crs_to_crs(),
and add (or undo) the needed axis swap operations so that the
object returned by proj_normalize_for_visualization() has the usual
GIS axis order.
In this implementation, this does something only if the coordinate
system of the source or target CRS, geographic or projected, has
NORTH, EAST ordering.
CompoundCRS wrapping those objects are also handled.

Question: should this be backported to 6.0.x ?

@rouault rouault force-pushed the proj_normalize_for_visualization branch from 990e1c5 to d4fc128 Mar 28, 2019
@kbevers
Copy link
Member

@kbevers kbevers commented Mar 28, 2019

In this implementation, this does something only if the source or
target CRS have latitude,longitude ordering.

But it is not limited to only coordinate systems with geodetic coordinates, right? The code seems to be checking projected coordinates as well but from the documentation it seems as if it only applies when latitudes and longitudes (i.e. angular coordinates) are involved.

Question: should this be backported to 6.0.x ?

No. This adds a new function so should not be included in a bug-fix release.

Loading

@rouault
Copy link
Member Author

@rouault rouault commented Mar 28, 2019

But it is not limited to only coordinate systems with geodetic coordinates, right? The code seems to be checking projected coordinates as well but from the documentation it seems as if it only applies when latitudes and longitudes (i.e. angular coordinates) are involved.

ah sorry, the commit message is wrong. I'm going to fix it. It does the inversion for geographic or projected CRS with axis order direction NORTH, EAST.

Loading

@rouault rouault force-pushed the proj_normalize_for_visualization branch from d4fc128 to d658ed1 Mar 28, 2019
@kbevers kbevers added this to the 6.1.0 milestone Mar 28, 2019
@nyalldawson
Copy link
Contributor

@nyalldawson nyalldawson commented Mar 28, 2019

I'd like to humbly request that this IS backported to 6.0 -- without it you're requiring all clients to implement their own version of https://github.com/pramsey/postgis/blob/7ecf6839c57a838e2c8540001a3cd35b78a730db/liblwgeom/lwgeom_transform.c#L299 in order to correctly project x/y coordinates. I think it's MUCH safer to have this sitting in the underlying proj library then have multiple varying versions of this test floating out their in the client applications.

Loading

Fixes OSGeo#1301

This function takes the output PJ from proj_create_crs_to_crs(),
and add (or undo) the needed axis swap operations so that the
object returned by proj_normalize_for_visualization() has the usual
GIS axis order.
In this implementation, this does something only if the coordinate
system of the source or target CRS, geographic or projected, has
NORTH, EAST ordering.
CompoundCRS wrapping those objects are also handled.
@rouault rouault force-pushed the proj_normalize_for_visualization branch from d658ed1 to 6a7e24d Mar 28, 2019
@kbevers
Copy link
Member

@kbevers kbevers commented Mar 29, 2019

I think it's MUCH safer to have this sitting in the underlying proj library then have multiple varying versions of this test floating out their in the client applications.

I absolutely agree. I am just not very keen on deviating from semantic versioning rules. The simple solution is to release 6.1.0 right away instead of releasing a few patch releases first. Thoughts?

Loading

@rouault
Copy link
Member Author

@rouault rouault commented Mar 29, 2019

The simple solution is to release 6.1.0 right away instead of releasing a few patch releases first. Thoughts?

So the 6.0.1 release of May 1st would become a 6.1.0 ? I'm fine with that.

Loading

@kbevers
Copy link
Member

@kbevers kbevers commented Mar 29, 2019

So the 6.0.1 release of May 1st would become a 6.1.0 ? I'm fine with that.

Yes. It would also fit well with the changes to the database. Are there any other functions that would be beneficial to add before May 1st?

Loading

@rouault rouault merged commit 8d7a01b into OSGeo:master Mar 29, 2019
3 checks passed
Loading
@kbevers kbevers removed this from the 6.2.0 milestone Apr 2, 2019
@kbevers kbevers added this to the 6.1.0 milestone Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants