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

coordinates return a Vector of Vector #20

Closed
mkborregaard opened this issue Jan 29, 2019 · 14 comments
Closed

coordinates return a Vector of Vector #20

mkborregaard opened this issue Jan 29, 2019 · 14 comments

Comments

@mkborregaard
Copy link
Contributor

Why not return a Vector of Tuples? Given that points are immutable. Tuples are stack allocated and much faster.

@yeesian
Copy link
Member

yeesian commented Jan 29, 2019

How might we handle jagged arrays using a Vector of Tuples?

@mkborregaard
Copy link
Contributor Author

mkborregaard commented Jan 29, 2019

I might be misunderstanding the spec, but I meant each 2d or 3d point to be represented by a Tuple in the returned object (the innermost Array) - the outer ones can have variable length and should be arrays. Or am I getting this wrong?

@yeesian
Copy link
Member

yeesian commented Jan 29, 2019

Yes, that should be possible. I just kept it to vectors for simplicity of implementation to get something going.

@mkborregaard
Copy link
Contributor Author

I'll try to see if I can work this out, but it looks like the whole interface expects Vectors at the lowest level, so this would need changing lots of places, right?

@juliohm
Copy link
Member

juliohm commented Jan 29, 2019

I suggest to use a StaticArrays.jl instead like I do in GeoStats.jl. Every coordinate is stack-allocated and still works in the context of linear algebra calculations. Basically a tuple on steroids.

@yeesian
Copy link
Member

yeesian commented Jan 29, 2019

I suggest to use a StaticArrays.jl instead like I do in GeoStats.jl. Every coordinate is stack-allocated and still works in the context of linear algebra calculations. Basically a tuple on steroids.

Sure, that works too.

I'll try to see if I can work this out, but it looks like the whole interface expects Vectors at the lowest level, so this would need changing lots of places, right?

Should be doable for this package, since I've isolated it to

const Position = Vector{Float64}

More preferably, we should be using a holy-traits approach to representing the Positions such that Vectors and Tuples and StaticArrays (and others) can all work.

@mkborregaard
Copy link
Contributor Author

That is another option - or perhaps even better using GeometryTypes.Point which is just a StaticArray.
It's just a question of that I don't know exactly what the intention is here - Tuples are the most assumption-free. What do you use linear algebra for on Points?

But definitely, I think a common abstraction for Points in Julia would be great. See also JuliaPlots/Plots.jl#1886 (comment)

@mkborregaard
Copy link
Contributor Author

Why traits and not just using the same type? Nothing is simpler than Point.
@yeesian no I did try to change that line, but the implementation in Shapefile/geo_interface.jl does not use the GeoInterface interface! It reimplements coordinates as Vectors.

@yeesian
Copy link
Member

yeesian commented Jan 29, 2019

Why traits and not just using the same type? Nothing is simpler than Point.

There's https://github.com/JuliaGeometry/meta/wiki/First-Meetup and https://github.com/JuliaGeometry/meta/wiki/First-Meetup-Minutes

the implementation in Shapefile/geo_interface.jl does not use the GeoInterface interface! It reimplements coordinates as Vectors.

Yup, that's the only hard part about it, no? Changing what an interface expects should be easy. What is more tedious is updating all the packages that implements it.

@juliohm
Copy link
Member

juliohm commented Jan 29, 2019

I think that by using StaticArrays.jl, which is quite standard nowadays, we gain a lot of functionality for free. Reimplementing all the same operations for Point or any other type wouldn't be productive. A generic traits-based interface also sounds like a good path forward.

I think the crux here is to keep in mind that points are just "vectors" and that people will definitely make linear algebra calculations with them.

@yeesian
Copy link
Member

yeesian commented Jan 29, 2019

I think that by using StaticArrays.jl, which is quite standard nowadays, we gain a lot of functionality for free. [...] A generic traits-based interface also sounds like a good path forward.

They are not mutually exclusive. The specific proposal is to

  1. Replace

    const Position = Vector{Float64}
    with a "blessed type" (e.g. GeometryTypes.Point), which it seems is going to be from StaticArrays.jl.

  2. Move towards a GeoInterface spec that is not tied to Vector or SVector or Float64, but is based on traits for retrieving coordinates at different indices.

@maxfreu
Copy link

maxfreu commented Sep 14, 2020

Have you made a decision on this?

@visr
Copy link
Member

visr commented Sep 14, 2020

Yes, it's going to be the number 2 above, see also #33, #26. There is no 'blessed' type like mentioned under number 1, but we try to encourage using the types from GeometryBasics if possible.

@evetion
Copy link
Member

evetion commented May 16, 2022

Closed by the traits interface of #33.

@evetion evetion closed this as completed May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants