multipolygon rotate pivot

[mapoulos]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

[michaelkirk]

I'm curious, does this still pass if you delete the max_relative=1.0?

My understanding of max_relative=1.0 is that you're allowing up to 100% error, which seems unnecessarily large.

[mapoulos]

Removing the max_relative does indeed fail:

running 1 test
thread 'algorithm::rotate::test::test_rotate_multipolygon_around_centroid' panicked at 'assert_relative_eq!(multipolygon.rotate(-90.), expected, max_relative = 0.1)

    left  = MultiPolygon([Polygon { exterior: LineString([Coordinate { x: 0.0, y: 0.0 }, Coordinate { x: 0.0000000000000006123233995736766, y: -10.0 }, Coordinate { x: 10.0, y: -10.0 }, Coordinate { x: 10.0, y: 0.0000000000000006123233995736766 }, Coordinate { x: 0.0, y: 0.0 }]), interiors: [] }, Polygon { exterior: LineString([Coordinate { x: 0.0, y: 0.0 }, Coordinate { x: -0.0000000000000006123233995736766, y: 10.0 }, Coordinate { x: -10.0, y: 10.0 }, Coordinate { x: -10.0, y: -0.0000000000000006123233995736766 }, Coordinate { x: 0.0, y: 0.0 }]), interiors: [] }])
    right = MultiPolygon([Polygon { exterior: LineString([Coordinate { x: 0.0, y: 0.0 }, Coordinate { x: 10.0, y: 0.0 }, Coordinate { x: 10.0, y: -10.0 }, Coordinate { x: 0.0, y: -10.0 }, Coordinate { x: 0.0, y: 0.0 }]), interiors: [] }, Polygon { exterior: LineString([Coordinate { x: 0.0, y: 0.0 }, Coordinate { x: -10.0, y: 0.0 }, Coordinate { x: -10.0, y: 10.0 }, Coordinate { x: 0.0, y: 10.0 }, Coordinate { x: 0.0, y: 0.0 }]), interiors: [] }])

My knowledge of floating point comparison is sketchy enough to where I'm not sure what tuning param is appropriate. It's possible I've just missed something stupid tho, so I'll look more tomorrow (maybe the polygons aren't right for what I'm trying to verify).

[michaelkirk]

Ignoring the small errors for now (which indeed might be some floating point rounding), a quick glance shows the coords seem to be way off.

e.g. look at the second coord of the first poly:

left.0[0].exterior[1]: Coordinate { x: 0.0000000000000006123233995736766, y: -10.0 }
right.0[0].exterior[1]: Coordinate { x: 10.0, y: 0.0 }

One thing that could be going on... be aware that the equals check is not a topology check — to be eq, the MultiPolygons can't just have the same shape, but also their member Polygons must be in the same order, and each members coords must be in the same order.

Maybe something going on there?

[mapoulos]

Thanks for the pointers. I tried to short cut my way to a rotation I thought I could do by hand but I either botched the expected or missed something subtle. I think I'll create a real example and then run it through geos to get the expected values.

[michaelkirk]

Thanks for the PR!

It mostly looks good, but I had some minor nits / questions.

[mapoulos]

Okay, I've use shapely as an oracle for a multipolygon rotation. I still relaxed the epsilon a tad in the assert_relative_eq! macro call but just a tad so I can write 0. instead of something like 0.000000000000887.

I've also added a test to ensure an un-centroid-able multipolygon returns itself. should we do the same for LineString and Polygon? Those both panic if a centroid can't be calculated. For instance, this code produces a panic:

let empty_polygon: Polygon<f64> = polygon![];
let rotated_empty_polygon = empty_polygon.rotate(90.);

[michaelkirk]

bors r+

[michaelkirk]

Those both panic if a centroid can't be calculated. For instance, this code produces a panic:

That sounds undesirable, but it seems like that's existing behavior, unaffected by this PR, so I'm fine with merging this as is.

I've added #652 to track the bug you uncovered. Feel free to work on it =)

[bors]

Build failed:

• ci result

[michaelkirk]

Build failed

Shoot, at first glance it looks like some failure in our testing code, unrelated to your change.

Unfortunately I don't have more time to dig into this right now.

[michaelkirk]

Build failed

Ok, I took a quick peak actually. It looks like arbitrary a crate we use for testing updated their minimum supported rust version to be something newer than what we're running in CI.

I can take a look at revving CI sometime in the next few days.

[urschrei]

@michaelkirk I should have some time over the weekend. Will ping you if I get to it.

[mapoulos]

I more or less assume there's nothing that I can or should do to fix the CI, but if I can help with something let me know.

[michaelkirk]

I'm loooking at CI now. It takes a while to build everything, but is pretty low effort provided everything "Just works"... I'll keep you updated.

[michaelkirk]

@mapoulos - I've fixed CI. Would you mind rebasing or merging in master and we can give it another go?

[mapoulos]

@michaelkirk done!

[michaelkirk]

bors try

[bors]

try

Build succeeded:

• ci result

[michaelkirk]

Thanks @mapoulos!