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
Refactor transportation layer's z12-14 logic #1207
Refactor transportation layer's z12-14 logic #1207
Conversation
Results evaluating commit 6dd9af5 (merged with base d19aa5b as 0530f85). See run details. PostgreSQL DB size in MB: 2792 ⇒ 2791 (-0.0% change)
expand for details...
|
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 detailed review
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.
This now very complex too, I don't know if it makes sense to change from complex to complex and then add maybe some unwanted bugs
) | ||
AND | ||
CASE WHEN zoom_level = 12 THEN | ||
CASE WHEN highway IN ('unclassified', 'residential') THEN TRUE |
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.
AND man_made <> 'pier'
is missing
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.
Are you saying for the case of highway=unclassified
+ man_made=pier
? I don't see that being a necessary check. If it's tagged as unclassified/residential, adding pier tagging shouldn't pull it out of z12.
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.
It was in the old sql checked. I don't know if it makes sense. But if you say it is not necessary, then I believe you.
Okay, I agree I haven't really solved the "too confusing" problem here, let me have another go at it. |
b5919ee
to
47666c3
Compare
Okay, I think I've got it sorted. Hopefully this is easier to look at this time. I was able to compact the changes and also come up with a cleaner solution to the |
47666c3
to
8e47c23
Compare
Great work! Thank you very much! |
This PR is a refactor. The purpose of this PR is to refactor the logic in the z12-z14 transportation layer to increase readability and reduce code complexity.
In addition, this fixes bugs in the original filter code:
service
andservice_construction
highway classes.class
valuesminor_construction
,path_construction
,service_construction
, andtrack_construction
from zoom 12 filter by explicitly listing the highway classes that are included.