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

Introduce OsmModel and cleanup GeoUtils #150

Open
2 of 4 tasks
johanneshiry opened this issue Nov 24, 2021 · 2 comments
Open
2 of 4 tasks

Introduce OsmModel and cleanup GeoUtils #150

johanneshiry opened this issue Nov 24, 2021 · 2 comments
Assignees
Labels
bug Something isn't working dependencies Pull requests that update a dependency file enhancement New feature or request
Milestone

Comments

@johanneshiry
Copy link
Member

johanneshiry commented Nov 24, 2021

Close PR of #133 first

The GeoUtils are not tested nor properly implemented at the moment. Furthermore, it would make sense to have a specific OsmModel in order to be independent of third party libraries.

@johanneshiry johanneshiry added bug Something isn't working enhancement New feature or request dependencies Pull requests that update a dependency file labels Nov 24, 2021
@johanneshiry johanneshiry added this to the Version 1.6 milestone Nov 24, 2021
@t-ober t-ober self-assigned this Nov 29, 2021
@larslenssen
Copy link
Collaborator

larslenssen commented Dec 3, 2021

I would like to discuss the structure of Relation and trait Way in OsmModel.

Is it necessary to store a list of nodes, is it not enough to store a list of IDs? Otherwise we will have duplicate information stored in the model. We could also consider numbering the nodes independently in ascending order. This way we could store the nodes as an array and access them faster.

Let's discuss!

@t-ober
Copy link
Contributor

t-ober commented Dec 4, 2021

I don't think we have duplicate information now as the nodes that are stored in the Way aren't a copy of the Nodes themselves but a reference to the same object. Not sure why we would explicitly number them as they have an associated index when being stored within a Sequence. If accessing a specific node somewhere in the list by their index is a usecase (although I'm not quite sure why) we should store them in a Vector rather than in a List (as Lists in Scala are implemented as linked list so accessing an by index is quite expensive). If we end up looking for specific Nodes (via their uuid or osmId) we should consider keeping a Map[UUID, Node]

@ckittl ckittl modified the milestones: Version 1.6, Version 2.0 Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants