Skip to content
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

Allow options to ArchGDAL driver when saving Geotiff #30

Closed
gottacatchenall opened this issue Oct 27, 2022 · 4 comments · Fixed by #47
Closed

Allow options to ArchGDAL driver when saving Geotiff #30

gottacatchenall opened this issue Oct 27, 2022 · 4 comments · Fixed by #47

Comments

@gottacatchenall
Copy link
Member

gottacatchenall commented Oct 27, 2022

Description of the to-do item

support for different drivers when writing geotiff

More information about the item

The current geotiff function for writing files uses the GTiff driver in ArchGDAL by default.

https://github.com/PoisotLab/SimpleSDMLayers.jl/blob/dcc49145c2c844bfe63fff99b96cf86720076512/src/datasets/geotiff.jl#L191

It would be nice to have this either be a keyword argument or something to enable selection of different drivers, e.g. I need to use the "COG" driver to write a cloud-optimized geotiff

@gottacatchenall gottacatchenall changed the title optinons for ArchGDAL driver when saving Geotiff ✅ options for ArchGDAL driver when saving Geotiff ✅ Oct 27, 2022
@tpoisot
Copy link
Member

tpoisot commented Oct 27, 2022

Yeah that's a good idea, and probably something that we need to abstract away from this package (I'm in the process of re-designing the data download/load/write layer in another repo). Meanwhile, a PR with a keyword argument is probably the most effort-effective solution?

@tpoisot tpoisot closed this as completed Nov 14, 2022
@tpoisot tpoisot transferred this issue from PoisotLab/SimpleSDMLayers.jl Nov 14, 2022
@tpoisot tpoisot reopened this Nov 14, 2022
@tpoisot tpoisot changed the title options for ArchGDAL driver when saving Geotiff ✅ Allow options to ArchGDAL driver when saving Geotiff Nov 14, 2022
@tpoisot tpoisot removed their assignment Nov 14, 2022
@tpoisot
Copy link
Member

tpoisot commented Nov 14, 2022

@gottacatchenall what do you think is the best way to do this? As in, what is the code you would like to write to make this happen? This is most likely the next issue I'll address.

@gottacatchenall
Copy link
Member Author

gottacatchenall commented Nov 14, 2022

my general thoughts are

the current syntax is both geotiff for reading and writing. i think geotiff(SimpleSDMPredictor, "input.tif") to load and geotiff(layer, "output.tif") to save could be a bit confusing, primarily because geotiff(SimpleSDMPredictor, "input.tif") goes (sink, source) in argument order which goes against what similar methods, e.g. CSV.read("filename.csv", DataFrame), which go (source, sink) in the argument order.

if it's possible to overload read and write, and then deal with the type of file written based on the extension of the string, that could be an alternative.

there could be some utility of having multiple aliases for the same function, i.e. methods on read and write and geotiff as well as more verbose method names, e.g. write_geotiff, the tradeoff being both dispatch time and namespace pollution

as for the engine, i think it only needs to be passed as a string to the ArchGDAL function for writing, so just a keyword argument driver to the saving function seems like the easier solution, although it may be nice to have an enum for many of the most common drivers used to save tifs


On a separate note, is the goal here to eventually have this ensemble of packages become something like SpeciesDistributionModels.jl? It seems a suite of simple OTS models with methods for computing validation metrics and visualization (plus some of the occupancy models from MetapopulationDynamics.jl) would put us at feature parity with spOccupancy

@tpoisot
Copy link
Member

tpoisot commented Nov 14, 2022

I am going to create a new issue about the confusion in the names (#38). Ideally, we can solve the "reading" part with another type constructor, and the saving part with an overload of save and a detection of the file format.

I like the idea of an argument to the driver, so that's a single keyword argument with the default as-is. This should be simple enough to solve.

As for the last point -> #37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants