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

Bugfix in CRS::alterCSLinearUnit for DerivedProjectedCRS #3499

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

hernando
Copy link
Contributor

@hernando hernando commented Dec 7, 2022

The implementation was calling alterUnit on the CS of the base projected CRS, but this CS could be different than that of the derived projected CRS being modified (e.g. the axes could be swapped).

The fix consist on altering the unit of the derived projected CRS, but only if the CS can be cast to Cartesian. It's doesn't look like other CS types need to be considered.

  • Tests added
  • Added clear title that can be used to generate release notes

Copy link
Member

@rouault rouault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/iso19111/crs.cpp Show resolved Hide resolved
src/iso19111/crs.cpp Outdated Show resolved Hide resolved
@hernando hernando force-pushed the fix-alter-cs-linear-unit branch 2 times, most recently from fd82392 to 031f506 Compare December 7, 2022 14:47
@jjimenezshaw
Copy link
Contributor

CC @jjimenezshaw

Thanks @hernando . Good catch about the axis order. Good that createDerivedProjectedCRSNorthingEasting test helper is used again.
And thanks @rouault for the OrdinalCS reference.

The implementation was calling alterUnit on the CS of the base
projected CRS, but this CS could be different than that of the
derived projected CRS being modified (e.g. the axes could be
swapped).

The fix consist on altering the unit of the derived projected
CRS, but only if the CS can be cast to Cartesian. It's doesn't
look like other CS types need to be considered.
@hernando hernando force-pushed the fix-alter-cs-linear-unit branch from 031f506 to 2d82ad0 Compare December 7, 2022 14:50
@rouault rouault added this to the 9.2.0 milestone Dec 8, 2022
@rouault rouault merged commit 8ccfea6 into OSGeo:master Dec 8, 2022
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.

3 participants