Add M-dim bindings

[JmsPae]

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

---

This PR seeks to add bindings for GDAL functions relating to higher-dimension geometry, in particular Measured geometry, with some other minor fixes/additions.

Added methods for Geometry:

• add_point_zm
• add_point_m
• set_point_zm
• set_point_m
• get_point_zm
• get_point_vec_zm
• iso_wkt
• iso_wkb

Considering renaming get_point_vec_zm to get_point_zm_vec, though neither exactly roll off the tongue.

Also modified the Debug impl. for Geometry to use iso_wkt in cases of measured geometry.

[JmsPae]

Maybe instead of a separate function, modifying the original wkb/wkt methods would be nicer, though that would be a breaking change.

[lnicola]

Quick r? @weiznich if you have time.

[weiznich]

Looks ok for me beside the inconsistencies around the index type. Those are not critical, but it would be nice to see them addressed.

[lnicola]

I noticed those, but they match the existing get_point and set_point.

But in #581 I went the other way around (i32 to usize)..

[JmsPae]

I noticed those, but they match the existing get_point and set_point.

But in #581 I went the other way around (i32 to usize)..

Should I just harmonize all these methods to use u32 where appropriate?

[lnicola]

I mean, u32 feels worse than both since it doesn't match the C type (and you can pass in a value larger than i32::MAX which overflows), and it doesn't play that well with idiomatic Rust code, which probably has an usize around.

[JmsPae]

We could do a checked conversion from usize to i32/c_int (eg. try_from) but then we'd have to make the methods fallible, which also isn't that great.

[lnicola]

LGTM with one last nit. No strong opinion on the i32 / usize thing though 🤷.

[lnicola]

PS: please squash at the end.

[JmsPae]

LGTM with one last nit. No strong opinion on the i32 / usize thing though 🤷.

IMHO it makes more sense to go with i32 if we're not (for now) making the methods fallible. I don't think the corresponding C functions do anything if you supply an index outside the range of points, which doesn't really give us a reason to throw any Err's unless we want to do any extra checks ourselves.

[lnicola]

Yeah, the GDAL functions log an error and return 0 (for GetPoint), so we'd have to check. But you still have to cast:

let points = todo!().
for (idx, p) in points.into_iter().enumerate() {
    geom.set_point_2d(idx as i32, p); // ugly, annoying for a new Rust user, and Clippy complains on clippy::pedantic
}

It's not too bad, though, more of a paper cut.

[JmsPae]

Something should probably be done, though it's starting to feel more out of scope for this PR. Otherwise, we can create a new Error type (the IntConversionError is too vague), make all the get/set methods fallible and accept usize/u32/whatever instead.

[lnicola]

Yeah, okay, let's leave them like this or use i32 for both (updating the changelog). But then what to do about #581?

[JmsPae]

This was probably a larger can of worms than we thought, eh?

I'm going to leave things as is for now and someone (maybe me in the not-so-distant future) can have a crack at a half-way decent solution.

I'll convert everything in geometry.rs to i32 since we're already meddling with it.

Never mind, that messed with way too much for this PR.

[JmsPae]

I feel like I have nothing else to add, so feel free to send it whenever.

[lnicola]

Yeah, can you please squash everything together?

[JmsPae]

There, pretty sure I've squashed everything now

[lnicola]

That was cute 😂, but for future reference: https://www.baeldung.com/ops/git-squash-commits.

[JmsPae]

That was cute 😂, but for future reference: https://www.baeldung.com/ops/git-squash-commits.

Ye, realized that, apologies.

Pretty sure this was also my first PR merge so now I know, and knowing is half the battle etc.

[lnicola]

Pretty sure this was also my first PR merge

It was really well-done, I wouldn't have guessed, good job! 😀