ArchGDAL.jl icon indicating copy to clipboard operation
ArchGDAL.jl copied to clipboard

higher level StringList option alternative like Dict

Open visr opened this issue 4 years ago • 6 comments

At several points in GDAL a StringList is expected for options. I somehow always forget what it should look like. Do you think it would be a good idea to accept Dicts or some other key/value datatype there instead?

These kind of options don't feel very julian: options = ["GEOM_POSSIBLE_NAMES=point,linestring", "KEEP_GEOM_COLUMNS=NO"], from https://yeesian.com/ArchGDAL.jl/latest/tables/.

Not sure if we discussed this before, though I see an old comment here about taking in Dicts in higher level functions: https://github.com/JuliaGeo/GDAL.jl/pull/3#issuecomment-149029236

Provide higher-level julia functions (auto-generated or handwrapped) that take-in and emit julian types (Dict/Vector/UTF8String/etc), that we'll have to write documentation for.

This kind of came up in https://github.com/evetion/GeoDataFrames.jl/issues/14.

Looking at how rasterio does creation options for instance, there are a lot of these for GeoTIFF: https://gdal.org/drivers/raster/gtiff.html#creation-options And can be passed as keyword arguments like this: https://rasterio.readthedocs.io/en/latest/topics/image_options.html#creation-options

This goes a bit further than Dict{String, String} since it lowercases the keys and maps "YES" to true etc, so we'd have to think if this can be done automatically to prevent the extra maintenance of these option mappings.

visr avatar May 17 '21 07:05 visr

Do you think it would be a good idea to accept Dicts or some other key/value datatype there instead?

Definitely.

As a proposal: I'm curious what you think of having NamedTuples or keyword args (c.f. #187)?

yeesian avatar May 20 '21 06:05 yeesian

so we'd have to think if this can be done automatically to prevent the extra maintenance of these option mappings.

Thanks for the extra thoughtfulness here and yeah, at first glance,I think the main requirements I can think of so far are:

  1. The names are correspondent to the GDAL option names.
    • If the GDAL option names are always going to be uppercase, then we can support case-insensitivity in the keyword names (by casting everything to uppercase before passing it to GDAL.
  2. The values can be marshaled to string in a straightforward fashion.
    • E.g. we can initially support e.g. String, Real, Bool and Vector{String} values and expand the set of types/possibilities over time based on what we learn?
  3. The documentation for the options can be easily updated
    • I think it might be difficult for us to auto-update the (handcrafted) docstrings here with the official documentation, so we could provide examples for a limited subset of options that are known to be stable over time, and point to the official (or GDAL.jl) documentation for other possibilities.

yeesian avatar May 20 '21 06:05 yeesian

This would be great, I've had a similar response to @visr a few times trying to work out how to pass options.

Keyword arguments feel the most Julian, but a Dict/NamedTuple is also a lot better than Vector{String}. Allowing lowercase would also feel more Julian than all caps.

Also consider allowing Symbol instead of String?

So commands could be something like:

...; geom_possible_names=(:point, :linestring), keep_geom_columns=false)

rafaqz avatar May 20 '21 07:05 rafaqz

Yes I think keyword arguments would probably be the nicest looking. One thing I wonder about for keywords, does it ever become problematic, them mixing with the existing keywords?

Right now we have:

read(filename; flags=OF_ReadOnly, alloweddrivers, options, siblingfiles)

Do we then do this?

read(filename; flags=OF_ReadOnly, alloweddrivers, siblingfiles, options...)

Such that the options still end up together in a single variable, and we don't mix them up with the other keyword arguments?

visr avatar May 20 '21 18:05 visr

Echoing https://github.com/yeesian/ArchGDAL.jl/issues/194#issuecomment-845368977, yeah I think all non-option keywords will have to be explicitly spelt out for those functions that accept options as kwargs.... There can also be a second version which is correspondent to the original function, taking in the options as a NamedTuple argument?

yeesian avatar May 28 '21 07:05 yeesian

Yeah it's probably possible to allow both.

rafaqz avatar May 28 '21 07:05 rafaqz