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

Unify PJ_OBJ and PJ structures #1208

Merged
merged 11 commits into from Dec 31, 2018

Conversation

Projects
None yet
3 participants
@rouault
Copy link
Member

rouault commented Dec 26, 2018

Implements what was discussed in https://lists.osgeo.org/pipermail/proj/2018-December/thread.html#8109
And also:

  • cleanup legacy structures of projects.h
  • merge projects.h into proj_internal.h
@kbevers

This comment has been minimized.

Copy link
Member

kbevers commented Dec 27, 2018

I think this generally looks very good. Is there still a purpose for the proj_obj_* namespace or are those leftovers?

@kbevers

This comment has been minimized.

Copy link
Member

kbevers commented Dec 27, 2018

Is there still a purpose for the proj_obj_* namespace or are those leftovers?

Same question for datatypes: PJ_OBJ_*.

@rouault rouault force-pushed the rouault:merge_PJ_and_PJ_OBJ branch from a48b193 to efcc2c4 Dec 28, 2018

@rouault

This comment has been minimized.

Copy link
Member

rouault commented Dec 28, 2018

Is there still a purpose for the proj_obj_* namespace or are those leftovers?

Same question for datatypes: PJ_OBJ_*.

Done, except for PJ_OBJ_LIST since there is a conflict with an existing struct PJ_LIST that is used for listing operations

@kbevers
Copy link
Member

kbevers left a comment

I am happy with this PR as it is now. There may be some minor details that I'll come across later but that's is not reason enough for holding this back. I am very happy that everything can be contained within the PJ struct.

@rouault

This comment has been minimized.

Copy link
Member

rouault commented Dec 29, 2018

@kbevers Should I merge this ?

@rouault rouault force-pushed the rouault:merge_PJ_and_PJ_OBJ branch from efcc2c4 to 9ed6659 Dec 29, 2018

@kbevers

This comment has been minimized.

Copy link
Member

kbevers commented Dec 29, 2018

@kbevers Should I merge this ?

Yes, if you are happy with it. I just didn't want to pull the trigger if you weren't ready yet :)

@snowman2 snowman2 referenced this pull request Dec 29, 2018

Open

update to proj.4 6.0.0 #155

rouault added some commits Dec 26, 2018

projects.h: remove #ifdef __cpluplus test since it must now be includ…
…ed from C++ file due to C++ objects in struct PJconsts

@rouault rouault force-pushed the rouault:merge_PJ_and_PJ_OBJ branch 2 times, most recently from de95ce4 to ae53784 Dec 30, 2018

rouault added some commits Dec 26, 2018

@rouault rouault force-pushed the rouault:merge_PJ_and_PJ_OBJ branch from ae53784 to 0e0e0e4 Dec 30, 2018

@rouault rouault merged commit 32f3ef4 into OSGeo:master Dec 31, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@snowman2

This comment has been minimized.

Copy link

snowman2 commented Jan 1, 2019

One thing that was nice about the separate namespace for the proj_obj versus just proj was that it differentiated the functions what went together.

For example. the PJ object created with proj_create does not play well with the other functions that used to have the proj_obj namespace. However, if the user uses proj_create_from_user_input (or a similar function), then the PJ object plays nicely with the other functions.

Thoughts on this?

@rouault

This comment has been minimized.

Copy link
Member

rouault commented Jan 1, 2019

@snowman2 Yes, I agree with that. We could perhaps improve that in the future, but it may be a bit risky for now to plug proj_create() and proj_create_argv() on proj_create_from_user_input () since the ISO191111 ingestion of PROJ string might be a bit lossy in its attempt to build a ISO19111 object if using some not so frequent PROJ keywords.

@snowman2

This comment has been minimized.

Copy link

snowman2 commented Jan 1, 2019

Makes sense - probably a good idea to keep it separate for now.

Do you think that it would be useful to:

  • Add an issue to add notes in proj.h for clarity to avoid confusion about the separation
  • Make an issue for potentially updating the behavior in the future when it makes sense
@rouault

This comment has been minimized.

Copy link
Member

rouault commented Jan 1, 2019

Do you think that it would be useful to:

* Add an issue to add notes in proj.h for clarity to avoid confusion about the separation

* Make an issue for potentially updating the behavior in the future when it makes sense

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment