-
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 raster creation options #193
Merged
jdroenner
merged 21 commits into
georust:master
from
geohardtke:add_raster_creation_options
Jun 22, 2021
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
ffff5f8
Added creation options support
geohardtke 657cd7b
Updated changelog
geohardtke cfc3866
Added test for create with options
geohardtke 23c4d8c
remove gdalinfo output
geohardtke b9a7a46
explicit drop of dataset before reading metadata
geohardtke ea83132
Applied rustfmt
geohardtke e412a59
Applied rustfmt
geohardtke 63a6743
Applied rustfmt
geohardtke 8d52fd4
pass options as array instead of vector
geohardtke ceb0bbc
remove unworthy comment
geohardtke 994bf2a
updated RasterCreationOption example
geohardtke 4113107
moved dataset creation to inner block
geohardtke 08170f8
Update RasterCreationOption example in CHANGES.md
geohardtke 49a0675
Improve description of the RasterCreationOption in CHANGES.md
geohardtke 0ea884c
Merge branch 'add_raster_creation_options' of https://github.com/geoh…
geohardtke 0e00b77
move RasterCreationOption to raste/mod.rs
geohardtke c385dd1
more idiomatic assert expression
geohardtke 5ab2e39
applied rustfmt
geohardtke 66fe328
Removed extra space
geohardtke 8533b0e
remove duplicate implementation
geohardtke 71ee1d9
Added link to PR
geohardtke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
shouldn't this be a *mut * mut char?
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.
You mean
c_char
?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.
yes,
options_c as *mut *mut i8
is correct. I'm not sure how it ended up beingnull_mut()
again, sorry.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 not
null_mut()
, it'soptions_c as *mut *mut i8
, jdroenner is asking if it should beoptions_c as *mut *mut c_char
.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.
oh, sorry! my bad. Blame Friday night.
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.
gdal_sys::GDALCreate expects *mut *mut i8.
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.
Yeah, it's defined as
char**
in the GDAL headers. We might be able to improve the bindings, but I think we shouldn't block merging this.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.
Any thoughts @jdroenner? Do the bindings need to be updated before merging this? I think I don't have enough knowledge to do that.
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.
no its fine. i just assumed it to be a char in the bindings.
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.
Cool! Thanks!