search

yeesian / ArchGDAL.jl

A high level API for GDAL - Geospatial Data Abstraction Library
https://yeesian.github.io/ArchGDAL.jl/stable/
Other
142 stars 26 forks source link

Implement MDArray API #433

Open eschnett opened 3 months ago

eschnett commented 3 months ago

See https://github.com/yeesian/ArchGDAL.jl/issues/432.

eschnett commented 3 months ago

I have implemented a sketch of the MDArray API. At the moment, everything is in one file and not yet nicely sprinkled into the existing structure.

I'm looking for feedback regarding the types and functions I'm defining. Am I on the right track? Am I missing some functionality or some convenient helper functions?

yeesian commented 3 months ago

I'm looking for feedback regarding the types and functions I'm defining. Am I on the right track? Am I missing some functionality or some convenient helper functions?

It looks great, thank you for working on this! I have a soft preference for having all the type definitions in https://github.com/yeesian/ArchGDAL.jl/blob/master/src/types.jl but am open to the way you have it right now if you feel strongly about it.

Question: Why do the I... types exist?

It is mostly to have information relevant to https://yeesian.com/ArchGDAL.jl/stable/memory/#Interactive-versus-Scoped-Objects at the type level.

eschnett commented 3 months ago

I am looking for advice how the difference in array index ordering (row- vs. column-major) and array index base(0 vs. 1) should be handled. Is there a precedent in ArchGDAL?

Specifically, the function read (https://gdal.org/api/raster_c_api.html#gdal_8h_1a894a28265a68e41ea02b7b401c739e92) that reads an MDArray into a Julia strided array takes an argument arraystartidx. Should this be 0-based or 1-based? Should the order of the values correspond to the Julia or the GDAL ordering?

eschnett commented 3 months ago

Is it okay to return an Group that has ptr==C_NULL as indicator of an error, or should I rather check for this condition and return a Union{Nothing,Group} instead? I am thinking e.g. of GetRootGroup (https://gdal.org/api/gdalmdarray_cpp.html#_CPPv4NK11GDALMDArray12GetRootGroupEv) here.

yeesian commented 3 months ago

I am looking for advice how the difference in array index ordering (row- vs. column-major) and array index base(0 vs. 1) should be handled. Is there a precedent in ArchGDAL?

The closest precedent I can think of is https://github.com/yeesian/ArchGDAL.jl/blob/4b40fe857503ec2ef8bc431919d8d31bcec7cba5/src/raster/rasterio.jl#L124-L169 (corresponding GDAL doc: https://gdal.org/api/raster_c_api.html#_CPPv421GDALDatasetRasterIOEx12GDALDatasetH10GDALRWFlagiiiiPvii12GDALDataTypeiPKi8GSpacing8GSpacing8GSpacingP20GDALRasterIOExtraArg)

i.e. (i) it'll be 0-based in index offsets (when receiving integers as index arguments) but to handle julian types like UnitRange as 1-based, (ii) there will be a version of the function that allows the user to specify all arguments, and optional arguments defaulting to what their GDAL default value might be (we'll use 0 or C_NULL where the GDAL default value is nullptr)

Is it okay to return an Group that has ptr==C_NULL as indicator of an error, or should I rather check for this condition and return a Union{Nothing,Group} instead? I am thinking e.g. of GetRootGroup (https://gdal.org/api/gdalmdarray_cpp.html#_CPPv4NK11GDALMDArray12GetRootGroupEv) here.

I think it is okay to return a Group that has ptr==C_NULL as indicator of an error. The precedent that I can think of is e.g. in OGR geometries https://github.com/yeesian/ArchGDAL.jl/blob/4b40fe857503ec2ef8bc431919d8d31bcec7cba5/src/ogr/geometry.jl#L512-L529

But I'm also open to being convinced otherwise (e.g. in https://github.com/yeesian/ArchGDAL.jl/pull/179#issuecomment-864720942 for https://github.com/yeesian/ArchGDAL.jl/issues/192)

eschnett commented 3 months ago

This concludes the first step of implementing support for multidimensional arrays.

Next I would like to receive feedback on design choices (if anyone is interested enough to care) and clean up things. This means

eschnett commented 2 months ago

@yeesian I'm getting back to this PR after being sidetracked for a while. Would you be open to a Zoom call to discuss how to proceed?

yeesian commented 2 months ago

@yeesian I'm getting back to this PR after being sidetracked for a while. Would you be open to a Zoom call to discuss how to proceed?

Yeah sounds good, thanks for picking this up again! I've emailed you (based on what I could find at your personal website) to arrange a date/time