Add test for Intersects predicate #5
Conversation
There was a problem hiding this comment.
There are a few test cases where the geometry seems to be in some sort of a base64 format
Maybe WKB?
Requires changes to wkt; I'm currently using a fork with the changes (support for POINT EMPTY, and LINEARRING).
I saw the PR for POINT EMPTY do you plan to make one for the LINNEARRING support as well? I'm anticipating incorporating jts-test-runner into the geo test suite pretty soon, and feel a little strange about having the geo test suite rely on a forked version of the WKT dep.
src/input.rs
Outdated
| pub cases: Vec<Case>, | ||
| } | ||
|
|
||
| #[derive(Debug, Deserialize)] | ||
| pub(crate) struct Case { | ||
| #[serde(default)] | ||
| pub(crate) desc: String, | ||
| // `a` seems to always be a WKT geometry, but until we can handle `POINT EMPTY` this |
There was a problem hiding this comment.
This comment can go away now
There was a problem hiding this comment.
There are a few tests that don't have a desc for some reason! Didn't want them to fail at parsing.
There was a problem hiding this comment.
Oh, I misread your comment (on the comment). Will remove it.
|
Nice work on this! I just want to understand your plan for WKT, but otherwise LGTM! |
Yeah, that was my guess too. There seems to be a rust-wkb crate, but what's confusing is there seems to be no flag that explains that the input is to be read as wkb. Let's address it in a future PR.
Yeah, the fork has some minimal support for rings, but wanted to discuss the issue further in the wkt repo before creating a PR. I do want to eventually move to using the standard repo. My priority was to support more traits in the tests, and then add the finishing touches. |
Yeah, totally happy to iterate on this.
In case you hadn't already noticed, I'm attempting to incorporate an earlier rev of this crate into geo as part of georust/geo#629, so if we merge this PR which requires a forked WKT, until that WKT fork gets upstreamed/released, we won't be able to update the jts-test-runner rev of That's fine with me for now - especially since this seems to be coming along pretty quickly, I just wanted to make sure we're on the same page. |
https://github.com/georust/geozero also has support for WKB. Even though I've used and contributed a couple times to that crate, I confess I don't completely grok it yet - it's sort of it's own universe, but seems very powerful. |
I'm totally okay with anyway you choose to bring these eventually into geo CI. Just wanted to test the current
Nice! geozero is indeed quite powerful in that it's zero-copy! |
Requires changes to wkt; I'm currently using a fork with the changes (support for POINT EMPTY, and LINEARRING). There are a few test cases where the geometry seems to be in some sort of a base64 format. Not sure how we should parse that.
All the parsed tests pass for hull, centroid, and intersects.