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

vgridshift: add a +multiplier={value} #1072

Merged
merged 1 commit into from Jul 11, 2018

Conversation

@rouault
Copy link
Member

@rouault rouault commented Jul 10, 2018

As mentionned in #1071, it is often unclear how the offset of a vertical grid
is applied.

@rouault
Copy link
Member Author

@rouault rouault commented Jul 10, 2018

Another possibility would be to add a +multiplier parameter, and then people would be able to control both direction and multiplier value (in case of non-meter units)

@kbevers
Copy link
Member

@kbevers kbevers commented Jul 10, 2018

I think the operation parameter is too easily confused with a "coordinate operation". Especially when used in gie files: operation proj = vgridshift grids = ... operation = add. I'm leaning towards the +multiplier parameter. It would also fix your original mm problem.

@rouault rouault force-pushed the rouault:vgridshift_add_operation_parameter branch from 78b2e7e to e8592d8 Jul 10, 2018
@rouault rouault changed the title vgridshift: add a +operation=add or +operation=substract parameter vgridshift: add a +multiplier={value} Jul 10, 2018
@rouault
Copy link
Member Author

@rouault rouault commented Jul 10, 2018

I'm leaning towards the +multiplier parameter.

Done

@rouault rouault force-pushed the rouault:vgridshift_add_operation_parameter branch from e8592d8 to 8bc7c00 Jul 10, 2018
As mentionned in #1071, it is often unclear how the offset of a vertical grid
is applied.
@rouault rouault force-pushed the rouault:vgridshift_add_operation_parameter branch from 8bc7c00 to b7afba5 Jul 10, 2018
@kbevers
Copy link
Member

@kbevers kbevers commented Jul 10, 2018

Looks good to me. We should reconsider if it makes sense to add the vertcon grids with adjusted units to proj-datumgrids.

@kbevers kbevers merged commit 98ead00 into OSGeo:master Jul 11, 2018
4 checks passed
4 checks passed
@travis-ci
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 77.778%
Details
@kbevers kbevers added this to the 5.2.0 milestone Aug 20, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants