TryFromWkt for integer geometries #103
Conversation
michaelkirk
force-pushed
the
mkirk/try-from-int-fail
branch
from
b80e597
to
9b92631
Compare
|
#101 has been merged, and this is ready for review |
michaelkirk
force-pushed
the
mkirk/try-from-int-fail
branch
from
9b92631
to
1ded04e
Compare
| @@ -38,15 +38,15 @@ pub enum Error { | |||
| }, | |||
| #[error("Wrong number of Geometries: {0}")] | |||
| WrongNumberOfGeometries(usize), | |||
| #[error("Invalid WKT")] | |||
| #[error("Invalid WKT: {0}")] | |||
There was a problem hiding this comment.
Note the InvalidWKT error message now includes the underlying error reason.
Cargo.toml
Outdated
|
|
||
| [dev-dependencies] | ||
| criterion = { version = "0.2" } | ||
| serde = { version = "1.0", default-features = false, features = ["derive"] } | ||
| serde_json = "1.0" | ||
| env_logger = "0.9.0" |
There was a problem hiding this comment.
I only used it locally. I'll remove it.
There was a problem hiding this comment.
Are you still planning on removing this?
There was a problem hiding this comment.
I think this is a stale diff - it was removed in 5cdefc6
| type_name::<T>() | ||
| ); | ||
| return Some(Err( | ||
| "Unable to parse input number as the desired output type", |
There was a problem hiding this comment.
This is probably not a number, right?
Actually, we might as well include it in the error instead of pulling log along.
There was a problem hiding this comment.
It is numberlike as per line 83. In particular to this PR is the case when trying to parse "1.23" as an integer, which will fail with this message.
The reason I logged here rather than building a String with the error details to the error is that errors throughout the WKT library are &'static str. I could do something different here, but it'd affect multiple places since this error is propagated.
There was a problem hiding this comment.
Seems like a good approach to me fwiw
michaelkirk
force-pushed
the
mkirk/try-from-int-fail
branch
from
f3574af
to
5cdefc6
Compare
|
Rebased since merging #99 |
Note that we error when trying to parse any numbers with decimal points with message: "Unable to parse input number as the desired output type" And log a more descriptive message like: [2022-05-18T19:26:16Z WARN wkt::tokenizer] Failed to parse input: '1.1' as i32 [2022-05-18T19:26:16Z WARN wkt::tokenizer] Failed to parse input: '1.9' as i32
michaelkirk
force-pushed
the
mkirk/try-from-int-fail
branch
from
5cdefc6
to
109ad71
Compare
|
bors r=urschrei |
|
Build succeeded: |
CHANGES.mdif knowledge of this change could be valuable to users.Fixes #102
This is based on #101, which @rmanoka already approved (thanks!), but I'm going to wait just a bit longer to merge it in case someone else wanted to take a look at it.update: #101 has been merged, and this is ready for review!