-
Notifications
You must be signed in to change notification settings - Fork 8
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
Document the centreline data #845
Conversation
Nudge @radumas |
@chmnata This is not meant to be complete; I've only been able to find out so much about this dataset, and currently there is no documentation that I'm aware of. This is just intended to be a start that we can add to later as time permits. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting a readme for centreline! Some comments below, and I will have to update it after merging to include some new changes on centreline_latest.
gis/centreline/readme.md
Outdated
|
||
## Nodes | ||
|
||
Nodes are stored in either of two tables, each of which is copied from a separate GCC layer which in turn are maintained by different groups. The differences between these tables is not completely understood. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I'll be able to give a good summary of the discussion there. Perhaps we could let Raha add to this documentation in a later PR?
Thanks @Nate-Wessel for the speedy fixes, @radumas wanna take a quick look at it? FYI I am gonna add the routing stuff in after this merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize there's a (this is about to be superseded by #885 ). Feel like this should instead be front and centre in the centreline
folder but I kiiiind of find it not that useful (anymore)gis/README.md
, though maybe it's a bit long. IDK.
There should be a reference somewhere that the code for the pipelines for these datasets are in gccview/
folder.
centreline_intersection_point
I would say is the preferred intersection layer, it is almost unique on intersection_id
intersection
appears to be 1 row to describe every relationship between edges at a node.
I think it would be helpful to add that intersection_id
is the ID in the intersection layer(s) that matches centreline_id
from MOVE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate this attempt to document the centreline.
As a centreline user recently what I feel is still lacking is some obvious pointers to use gis_core
over gis
, and centreline_latest
over centreline
.
- As part of this PR could we add comments to these two schemas and most of the tables under
gis_core
? - Add a link and overview of this readme under the main gis/readme.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvements to the documentation. Also thanks for adding comments on those tables in bigdata.
@radumas I addressed your comments from above. Can you review updates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding all the other folders description and more info on intersection! @gabrielwol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job team
What this pull request accomplishes:
Issue(s) this solves:
closes #801
What, in particular, needs to reviewed:
What needs to be done by a sysadmin after this PR is merged
NA