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

Definition parameter +no_defs crashes pyproj #22

Closed
ReallyNiceGuy opened this issue Oct 5, 2015 · 8 comments
Closed

Definition parameter +no_defs crashes pyproj #22

ReallyNiceGuy opened this issue Oct 5, 2015 · 8 comments

Comments

@ReallyNiceGuy
Copy link
Contributor

Any definition parameter without an '=' will actually crash the logic on init.py, line 698:
File "/usr/local/lib/python2.7/dist-packages/pyproj/init.py", line 698, in new
k,v = kvpair.split('=')

The definition I used was: +proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +units=m +nadgrids=@null +no_defs +x_0=5113560.997131 +y_0=2662158.879021

@micahcochran
Copy link
Collaborator

I'm running pyproj 1.9.4.

from pyproj import Proj, transform

p_grid = Proj('+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +units=m +nadgrids=@null +no_defs +x_0=5113560.997131 +y_0=2662158.879021')
p_wgs = Proj('+init=epsg:4326')
transform(p_wgs, p_grid, -87.6, 34.5, 0)

>>>  (-4626894.447280438, 6758297.919468232, 0.0)

I tried transforming a point to that projection. It gave the above output and didn't crash. I couldn't replicate a crash.

Could you give a code sample that demonstrates the crash? Or try the above code.

@ReallyNiceGuy
Copy link
Contributor Author

import pyproj
g=pyproj.Geod("+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +units=m
+nadgrids=@null +no_defs +x_0=5113560.997131 +y_0=2662158.879021")
Traceback (most recent call last):
File "", line 1, in
File "/usr/local/lib/python2.7/dist-packages/pyproj/init.py", line
697, in new
k,v = kvpair.split('=')
ValueError: need more than 1 value to unpack

Version 1.8.9, but the crashing code is the same

On Mon, Oct 5, 2015 at 6:26 PM, Micah Cochran notifications@github.com
wrote:

I'm running pyproj 1.9.4.

from pyproj import Proj, transform

p_grid = Proj('+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +units=m +nadgrids=@null +no_defs +x_0=5113560.997131 +y_0=2662158.879021')
p_wgs = Proj('+init=epsg:4326')
transform(p_wgs, p_grid, -87.6, 34.5, 0)

(-4626894.447280438, 6758297.919468232, 0.0)

I tried transforming a point to that projection and it gave me output and
didn't crash. I couldn't replicate this.

Could you give a code sample that demonstrates the crash?


Reply to this email directly or view it on GitHub
#22 (comment).

@micahcochran
Copy link
Collaborator

Using that Proj.4 definition in Geod() on version 1.9.4 raises a ValueError exception.

With the exception of +a= and +b= parameters, everything else in that string is nonsensical input for Geod(). What should pyproj when given input that is a valid Proj.4 string, but not valid for defining an ellipse? My hunch is that it should ignore the invalid parameter, which is what it seems to do for all the other Proj.4 parameters.

@ReallyNiceGuy
Copy link
Contributor Author

I guess that the library already ignores it. We should check if the
parameter contains an '=' before splitting it. I don't know what to do with
the parameter after that, as it expects a pair for every one.

On Tue, Oct 6, 2015 at 11:21 AM, Micah Cochran notifications@github.com
wrote:

Using that Proj.4 definition in Geod() on version 1.9.4 raises a
ValueError exception.

With the exception of +a= and +b= parameters, everything else in that
string is nonsensical input for Geod(). What should pyproj when given
input that is a valid Proj.4 string, but not valid for defining an ellipse?
My hunch is that it should ignore the invalid parameter, which is what it
seems to do for all the other Proj.4 parameters.


Reply to this email directly or view it on GitHub
#22 (comment).

@micahcochran
Copy link
Collaborator

Proj.__new__() only does .split('=') when preserve_units=False and only to get rid of the +units parameter in order to force the units to meters. Proj() feeds the Cython code a Proj.4 string, even if you gave dictionary input. I assume that string gets passed up to the Proj.4 C library, but I quit reading it--too much information.

Underneath the Geod Cython Code there is a C port of GeographicLib (natively a C++ library), which is included in the Proj.4 library. Interestingly enough, there is a Python port of GeographicLib, but it does not include other ellipses. I assume the intent of the Geod class is mirror the geod command. The Geod.__new__(), passes only what the values of +a and +f are to the Cython code.

Checking if the parameter contains a '=' before splitting is a great workaround. Would you like to code it or would you like someone else to code the workaround?

@ReallyNiceGuy
Copy link
Contributor Author

I can do it. I will just fork it and push the change to you.

On Tue, Oct 6, 2015 at 5:25 PM, Micah Cochran notifications@github.com
wrote:

Proj.new() only does .split('=') when preserve_units=False and only
to get rid of the +units parameter in order to force the units to meters.
Proj() feeds the Cython code a Proj.4 string, even if you gave dictionary
input. I assume that string gets passed up to the Proj.4 C library, but I
quit reading it--too much information.

Underneath the Geod Cython Code there is a C port of GeographicLib
http://geographiclib.sourceforge.net/ (natively a C++ library), which
is included in the Proj.4 library. Interestingly enough, there is a Python
port https://pypi.python.org/pypi/geographiclib of GeographicLib, but
it does not include other ellipses. I assume the intent of the Geod class
is mirror the geod command https://trac.osgeo.org/proj/wiki/man_geod.
The Geod.new(), passes only what the values of +a and +f are to the
Cython code.

Checking if the parameter contains a '=' before splitting is a great
workaround. Would you like to code it or would you like someone else to
code the workaround?


Reply to this email directly or view it on GitHub
#22 (comment).

@micahcochran
Copy link
Collaborator

Can this be closed?

@ReallyNiceGuy
Copy link
Contributor Author

I think so. The issue is fixed and there is a unit test for it.
On Jan 17, 2016 6:22 PM, "Micah Cochran" notifications@github.com wrote:

Can this be closed?


Reply to this email directly or view it on GitHub
#22 (comment).

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

2 participants