-
Notifications
You must be signed in to change notification settings - Fork 2
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
OSM tag format #7
base: master
Are you sure you want to change the base?
Conversation
See issue #3 for discussion.
Hello @ianthetechie! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-09-20 21:58:42 UTC |
The stuff with the EBNF is pretty cool! Though, I would have expected that in the end, it would be less code, but it isn't. Why though? |
It actually was a bit less code to do only the minimal parsing done before. However, I actually created a full EBNF grammar and accompanying parsing function so that we can transform this into multiple output formats and get much stronger validation. The original approach didn't do any serious transformation of the data (just some regex group matches, and couldn't do anything at all with components like restrictions). |
Ah, that makes sense. I am asking cause I am thinking about using an EBNF parser for the |
] | ||
"(default)": { | ||
"maxspeed": "60" | ||
} | ||
}, | ||
"urban single carriageway": { |
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.
First, maybe I am mixing something up, but should the road type name at this point not already be replaced with the tags from the second table or dropped if it cannot be mapped? Or is it just left as-is if it cannot be mapped?
(I know, we talked about maybe not mapping the road types to tags already in this file but instead in a second one to make it possible to inject a fuzzy mapping, I am just commenting on the state of the code as it is now. )
Second, with "urban single carriageway" road type specifically and some others, the mapping on the wiki page includes ???
. I put it this way into the wiki page rather than just leaving it for two reasons:
- so that one can immediately see on the page that some explicit key is missing
- because part of the keys it should map to are already clear, i.e. in this case only what constitutes "urban" is not clear, but the rest is:
??? and (oneway!~^yes|-1$ or dual_carriageway=no)
.
So the parser should probably either treat this as "can not map", or I change the wiki page to leave out the keys that can (partially) not be mapped out completely from the second table.
(This PR is obsolete now, I continued work on it on my fork) |
This will address issue #3.