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

Use `geometry` rather than `geom`

Open rafaqz opened this issue 1 year ago • 6 comments

GeoInterface.jl defaults to tables using geometry as the geometry column name. It would be nice if GADM.jl tables just worked in e.g. Rasters.jl without manually specifying the geometry column or getting them out.

See: https://github.com/JuliaGeo/GeoInterface.jl/blob/main/src/interface.jl#L47

This default can be overridden with a custom table type in GeoInterface.geometrycolumns. But for NamedTuple tables that requires type pyracy, so using the default column name is the way to get compatibility.

rafaqz avatar May 11 '23 11:05 rafaqz

@rafaqz, the name geom is defined by the ArchGDAL.jl package. The GADM.j code just converts the data returned by ArchGDAL.jl into a table using the Tables.columntable function.

eliascarv avatar May 16 '23 18:05 eliascarv

Its probably defined in the dataset itself, but gdal passes us :geom in GeoInterface.geometrycolumns so it can be called whatever they like.

GADM.jl strips that information by converting the table with Tables.columntable.

Without changing the column name to the generic one at the same time you lose ecosystem compatability.

Another option would be to define a wrapper table that knows which column has the geometries.

rafaqz avatar May 16 '23 19:05 rafaqz

Its probably defined in the dataset itself, but gdal passes us :geom in GeoInterface.geometrycolumns so it can be called whatever they like.

What is your suggestion then? We simply get whatever column name that GDAL returns to us and move it forward. I don't see what we could do differently? Why we need an extra wrapper table type if ArchGDAL.jl already provides that for us?

juliohm avatar May 16 '23 19:05 juliohm

But youre not returning the ArchGDAL table... thats where the problem comes from. If you did it would work, and would be ArchGDALs problem if it didnt.

From my perspective, the options are

  1. Return the table as-is from ArchGDAL (probably deleting 1 line of code)
  2. Change the column name to geometry in the conversion process (maybe adding 2 lines of code)
  3. Add your own wrapper table (probably annoying)
  4. Leave things as they are, and everyone has to manually do table.geom.

It would be nice not to do 4, its a bit confusing to new users to see that in code everywhere.

rafaqz avatar May 16 '23 19:05 rafaqz

A fifth options is to leverage the new metadata interface for tables, and add the column name to metadata so it propagates everywhere.

But its some serious work to implement that in GeoInterface and in every table producing geo package, and someone would need to put their hand up to do it.

Edit: And I dont think it propagates to the table youre returning anyway, so probably not an option.

rafaqz avatar May 16 '23 19:05 rafaqz

2. Change the column name to geometry in the conversion process (maybe adding 2 lines of code)

We should probably go with this option. If someone starts working on it, please comment on the issue.

juliohm avatar Jun 14 '23 11:06 juliohm