-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add RasterBand #24
Add RasterBand #24
Conversation
…asterband.rs. Added more rasterband functions and tests.
This looks awesome! Will review it later today |
} | ||
|
||
|
||
pub fn get_rasterband<'a>(&'a self, band_index: isize) -> Option<RasterBand<'a>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's Rust convention to not have get_
prefixes on getters, but it's up to you.
https://doc.rust-lang.org/style/style/naming/README.html#gettersetter-methods-rfc-344
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do decide to change this, there are other get_
cases below that could also be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe that's the price for working with multiple languages at the same time ;) I will remove all get_
prefixes.
Looks great! Feel free to merge. On a separate note, you seem to be leading the rust-gdal effort at the moment, so I'm going to leave it up to you for the direction of this library :) If I had more time, I'd love to also work on it, but I'm a bit busy currently. I'll still be happy to do reviews though |
I also just granted crates.io publish rights to the georust/core team, so you should be able to publish your changes whenever you want. |
Thanks :) I guess we will have to see how much time i can spend for rust-gdal. But i will try to do my best :) |
Latest commit looks good. Want me to merge and |
This adds a
struct RasterBand
and moves all related functions fromDataset
toRasterBand
.I'm not sure if the read/write functions in Dataset should be removed or stay as shortcuts. As requesting a
RasterBand
from aDataset
should return anOption<RasterBand>
i adapted the shortcuts to reflect this...Additionally i added some more Rasterband specific functions and implemented Metadata #20 for RasterBand.