Add more SpatialRef methods #145
Conversation
dmarteau
force-pushed
the
update_spatial_ref
branch
10 times, most recently
from
7a81851 to
b140f7c
Compare
dmarteau
force-pushed
the
update_spatial_ref
branch
from
b140f7c to
9ec6b78
Compare
CHANGES.md
Outdated
| @@ -1,5 +1,8 @@ | |||
| # Changes | |||
|
|
|||
| ## unreleased | |||
| * Add more functions to SpatialRef implementation | |||
There was a problem hiding this comment.
Looks like there's already an Unreleased heading, you can add this new item beneath it.
dmarteau
force-pushed
the
update_spatial_ref
branch
from
9ec6b78 to
379b5bc
Compare
| Ok(_string(c_ptr)) | ||
| } | ||
|
|
||
| pub fn get_angular_units(&self) -> f64 { |
There was a problem hiding this comment.
So if I want to get both the name and value I need to make two calls like this right?
let name = srs.get_angular_units_name();
let value = srs.get_angular_units();
Alternatively, you could consider making the api something like:
let (name, value) = srs.get_angular_units()
let name = srs.get_angular_units_name();
let value = srs.get_angular_units_value();
(I don't have any authority in whether or not this gets merged, I'm just a gdal crate user that watches PR's come by)
There was a problem hiding this comment.
uh well i guess both ways are fine. I would suggest to keep it this way to avoid mixing Results.
There was a problem hiding this comment.
It tooks me some time to end up with this solution but many of the use case I have faced up involved returning either only the name or only the value for calculation, since returning the name implies allocation it considered that it was better to have a zero cost way to get the units factor value (note: this is also the choice of the python bindings).
src/spatial_ref/srs.rs
Outdated
|
|
||
| pub fn get_axis_orientation(&self, target_key: &str, axis: i32) -> AxisOrientationType { | ||
| // We can almost safely assume that if we fail to build a CString then the input | ||
| // is not a valide key. |
There was a problem hiding this comment.
Should this be an Option/Result type? Or is it reasonable for users to infer that an error occurred when they get gdal_sys::OGRAxisOrientation::OAO_Other?
There was a problem hiding this comment.
you are right is should be a result type. The GDAL docs state that it will return " the name of the axis or NULL on failure. "
There was a problem hiding this comment.
That was not obvious to me since "No CPLError is issued on routine failures (such as not finding the AXIS)".
The error would then be a 'AxisNotFound' error value ?
In this case get_axis_name should also return a Result too not an Option<String> imho,
There was a problem hiding this comment.
Changed to Result return type for both get_axis_orientation and get_axis_name
|
Thank you @michaelkirk for reviewing and of cause thanks @dmarteau for the contribution. I have a lot to do at the moment so sometimes there is just no time for more code reviews. |
CHANGES.mdif knowledge of this change could be valuable to users.This PR add more bridge functions to gdal::spatial_ref::srs::SpatialRef implementation.
Struct added
Type alias
Functions added:
Added tests.