-
Notifications
You must be signed in to change notification settings - Fork 120
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
SQL to hide unwanted NE borders #2072
Conversation
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.
These all need to be visible for different audiences (mostly Russia, Turkey, Taiwan), but should be unrecognized
for the others.
Can you switch this around so it's modifying the fclass_*
attributes instead, please?
for more information, see https://pre-commit.ci
… into junderwood/suppress_ne_disputes
-- Add Russian disputed view to Nagorno-Karabakh | ||
ab_do_so_tr_ne_10m_admin_0_boundary_lines_land as ( | ||
update ne_10m_admin_0_boundary_lines_land l set | ||
fclass_ru = '' |
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.
Is the intent this is "default" which happens to be unrecognized
?
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.
Do we want this to be unrecognized? I thought we wanted to show a disputed border there for russia to show that they acknowledge something existing, but not quite a country.
update ne_10m_admin_0_boundary_lines_land l set | ||
fclass_tr = 'country' | ||
where l.ne_id in ( | ||
select ne_id from nagorno_karabakh |
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.
Is this for Northern Cyprus or nagorno_karabakh?
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.
Yeah good call. This isn't really ready for review yet. I'm just testing out the syntax
data/perform-sql-updates.sh
Outdated
@@ -43,6 +43,10 @@ echo "done." | |||
echo -e "\nDeleting disputed names" | |||
psql $PSQLOPTS $@ -f apply-planet_disputed_area_features_name_suppression.sql | |||
|
|||
# Deletes some of the Natural Earth dispute borders that we do not want to render |
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 is no longer deleting, please update comment and echo
…causing the problem. Refactored to simpler queries that run individually.
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 really like the direction of this updated PR.
This does point that we need to do a similar place=unrecognized
for OSM boundaries so we have consistency on zoom in. Can be followup PR, just link them together please.
This deletes any NE border feature that matches with the list of ne_ids in the
remove_lines
table.