Skip to content
This repository has been archived by the owner on Jul 4, 2022. It is now read-only.

try GeometryBasics metadata approaches #3

Closed
wants to merge 44 commits into from
Closed

try GeometryBasics metadata approaches #3

wants to merge 44 commits into from

Conversation

visr
Copy link
Owner

@visr visr commented Jul 3, 2020

It's nice to use this as a basis for exploring the GeometryBasics approach to metadata, right now for JuliaGeometry/GeometryBasics.jl#49.

Since GeoJSON is a well defined standard, we know exactly what we need. Currently in this PR the switch to GeometryBasics geometries has already been made. To explore the needs for a future metadata in GeometryBasics, I avoided using meta geometries for know, and simply used Feature as a composed geometry and named tuple. FeatureCollection currently holds a vector of Features, but should be switched to StructArrays as suggested in JuliaGeometry/GeometryBasics.jl#49.

Supersedes #2

src/GeoJSONTables.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@Sov-trotter
Copy link
Collaborator

Sov-trotter commented Jul 9, 2020

The current implemenation is starting to hold well for homogeneous types.
Wherein GeoJSONTables.Feature{GeometryBasics.Point{2,Int64},(:a, :b),Tuple{String,Int64}} comes out of println(f) in the staticschema method.

While for heterogeneous data Feature{T,(:a, :b),Tuple{String,Int64}} comes out or Feature{T,(:a, :b),Types} where Types where T
So basically we're getting T/Types instead of concrete types depending on whatever is heterogeneous with a
MethodError: no method matching getnamestypes(::Type{Feature{T,(:a, :b),Types} where Types where T}).

The this type inconsistency occurs while creating the vector of Feature structs -> Line - 34
I tried to overload the getnamestypes method with Any to get it working, but that didn't help either
@piever Do you think I am missing something here?

@visr
Copy link
Owner Author

visr commented Jul 9, 2020

I don't fully understand the issue for heterogeneous types. We are using Feature with type parameters to have the ability to represent both homogeneous (Feature{Point{2, Float64}}) and heterogeneous (Feature{Any/Union}) feature collections. The getnametypes error seems to be a misspelled function name, it should be getnamestypes.

src/SA_test.jl Outdated Show resolved Hide resolved
@piever
Copy link

piever commented Jul 9, 2020

With these types of custom types, you can't really expect StructArrays to know how to widen things correctly upon collection, or at least, I don't know a good way to do it in practice: how would StructArrays figure out that the first type parameter is linked to the first field? In principle, one could allow the widening mechanism to be customizable, but that is not implemented in StructArrays at the moment.

I think it's much easier to create the StructArray from a list of vectors. If you have a Feature iterator, you could use Tables.columntable (Tables is already a StructArrays dependency, so I guess it's OK to use it) on the iterator to change the storage to "column-based".

So something like

maketable(iter) = maketable(Tables.columntable(iter)::NamedTuple)

# assuming `geometry` is first in `propertynames(f::Feature)`
maketable(cols::NamedTuple) = maketable(first(cols), Base.tail(cols)) # you could also compute the types here with `Base.tuple_type_tail` and `Base.tuple_type_head`

function maketable(geometry, properties::NamedTuple{names, types}) where {names, types}
    F = Feature{eltype(geometry), names, StructArrays.eltypes(types)}
    return StructArray{F}(; geometry=geometry, properties...)
end

Note: this requires overload both propertynames and getproperty for Feature.

@Sov-trotter
Copy link
Collaborator

In principle, one could allow the widening mechanism to be customizable, but that is not implemented in StructArrays at the moment.

Not sure if it's correct, but I wonder if it was possible to write a collect_structarrays approach for the above method for non-standard data layouts?

@visr
Copy link
Owner Author

visr commented Jul 9, 2020

Let's keep it as simple as possible, and only focus on constructing it from a list of vectors for now.

@Sov-trotter
Copy link
Collaborator

Sov-trotter commented Jul 9, 2020

The new approach works well! For weird reasons I was getting a type NamedTuple has no field properties error.
Unknowingly I put everything in the old environment(one with the createinstance, staticschema methods) and the error was gone!
I had it tested a few times, and things work well, so the latest commits include a StructArrays.staticschema method. I wonder what must be the reason for it? Maybe it was supposed to be written this way? Will test it a few more times before finally integrating everything?

@Sov-trotter
Copy link
Collaborator

Screenshot from 2020-07-09 22-28-07

@visr visr mentioned this pull request Jul 10, 2020
src/GeoJSONTables.jl Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/GeoJSONTables.jl Outdated Show resolved Hide resolved
src/GeoJSONTables.jl Show resolved Hide resolved
src/GeoJSONTables.jl Outdated Show resolved Hide resolved
src/GeoJSONTables.jl Outdated Show resolved Hide resolved
src/GeoJSONTables.jl Outdated Show resolved Hide resolved
This was linked to issues Jul 25, 2020
@Sov-trotter Sov-trotter linked an issue Aug 9, 2020 that may be closed by this pull request
@visr
Copy link
Owner Author

visr commented Jun 24, 2022

Closing as we've decided to keep the wrapped JSON3 Objects as a kind of lazy geometry type, and implement GeoInterface on them, such that we can easily convert to GeometryBasics or other more useful geometry types.

@visr visr closed this Jun 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants