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

Adding more functions to the proj.h API #544

Merged
merged 5 commits into from
Jul 13, 2017
Merged

Conversation

kbevers
Copy link
Member

@kbevers kbevers commented Jul 12, 2017

The added functions are:

proj_create_crs_to_crs()
proj_obs()
proj_transform_obs()
proj_transform_coord()
proj_has_inverse()
proj_rtodms()
proj_dmstor()

Regarding proj_create_crs_to_crs(), I changed the parameter names to srid_* instead of def_*. I think that is a more helpful name in this particular context.

@kbevers kbevers merged commit ee7cb5d into OSGeo:master Jul 13, 2017
@kbevers kbevers added this to the 4.9.4 milestone Jul 13, 2017
Batch transform an array of PJ_OBS.

Returns 0 if all observations are transformed without error, otherwise
returns error number.
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Member Author

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:

  1. 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.

  2. 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?

Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Member

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.
Copy link
Contributor

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
Copy link
Contributor

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);
Copy link
Contributor

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.

Copy link
Member Author

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.

@kbevers kbevers deleted the api-functions branch July 14, 2017 06:34
@kbevers kbevers modified the milestones: 5.0.0-b, 5.0.0 Feb 1, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants