-
Notifications
You must be signed in to change notification settings - Fork 768
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
Adding more functions to the proj.h API #544
Conversation
Batch transform an array of PJ_OBS. | ||
|
||
Returns 0 if all observations are transformed without error, otherwise | ||
returns error number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be noted that obs
is now indeterminate; no way of knowing what's original or what's transformed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll take care of that when writing the actual docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking a bit more about this... At the moment the proj_transform_*
functions are very crude. I just threw them together quickly to fill out the blanks in the last API PR by @busstoptaktik. Error handling needs to be improved. I can think of two different solutions:
-
Mimic
pj_transform
. That's more or less what the functions do now, except they don't distinguish between transient and permanent errors. It should be easy to implement the transient/permanent bit. -
Return the number of succesfully transformed coordinates and let the user ask for the error number by calling
proj_errno
. In case different errors are returned during the loop the user will get the last returned error-number. Potentially that is a problem.
In both cases HUGE_VAL
's are returned for coordinates that fail to transform. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nowadays, wouldn't returning NaN be a better indication of a problem? HUGE_VAL is a reasonable valid result (a point that transforms to infinity). Also making sure that all the transformation routines deal with arguments which are NaN would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nowadays, wouldn't returning NaN be a better indication of a problem?
I think so, yes. What is the reason this hasn't been done in the past? Some compatibility thing? I was just going with the known behaviour of pj_transform
, but I'm fine with using NaN's instead. It might require thorough run-down of the existing code to make sure that it behaves in the same way throughout the code base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One potential reason that HUGE_VAL was selected in the past is that nan handling prior to C99 was a bit painful
Batch transform an array of PJ_COORD. | ||
|
||
Returns 0 if all coordinates are transformed without error, otherwise | ||
returns error number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here with coord
.
systems. | ||
|
||
srid_from and srid_to should be the value part of a +init=... parameter | ||
set, i.e. "epsg:25833" or "IGNF:AMST63". Any projection definition that is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra 'is'.
|
||
/* The shape of jazz to come! */ | ||
double proj_dmstor(const char *is, char **rs) { | ||
return dmstor(is, rs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defining these in the opposite direction would allow you to implement some kind of deprecation notice on the old names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I think we should agree on what's going to be deprecated, if anything at all.
The added functions are:
Regarding
proj_create_crs_to_crs()
, I changed the parameter names tosrid_*
instead ofdef_*
. I think that is a more helpful name in this particular context.