From 96a44c11542905a47f9b77595105cc43608bd310 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Tue, 10 Oct 2017 15:23:26 -0400 Subject: [PATCH 1/2] adds voyager basemaps as default basemap --- cartoframes/context.py | 2 +- cartoframes/layer.py | 29 ++++++++++++++++++++--------- cartoframes/maps.py | 3 ++- test/test_context.py | 11 ++++++----- test/test_layer.py | 16 ++++++++++++++++ 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/cartoframes/context.py b/cartoframes/context.py index e5885a976..60a6ded70 100644 --- a/cartoframes/context.py +++ b/cartoframes/context.py @@ -679,7 +679,7 @@ def map(self, layers=None, interactive=True, elif not base_layers: # default basemap is dark with labels in back # labels will be changed if all geoms are non-point - layers.insert(0, BaseMap(source='dark', labels='back')) + layers.insert(0, BaseMap()) geoms = set() # Setup layers diff --git a/cartoframes/layer.py b/cartoframes/layer.py index 821390a17..89a7ed09f 100644 --- a/cartoframes/layer.py +++ b/cartoframes/layer.py @@ -53,21 +53,31 @@ class BaseMap(AbstractLayer): """ is_basemap = True - def __init__(self, source='dark', labels='back', only_labels=False): + def __init__(self, source='voyager', labels='back', only_labels=False): if labels not in ('front', 'back', None): raise ValueError("labels must be None, 'front', or 'back'") self.source = source self.labels = labels + stem = 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' + if source == 'voyager': + stem += 'rastertiles' if self.is_basic(): if only_labels: style = source + '_only_labels' else: - style = source + ('_all' if labels == 'back' else '_nolabels') - - self.url = ('https://cartodb-basemaps-{{s}}.global.ssl.fastly.net/' - '{style}/{{z}}/{{x}}/{{y}}.png').format(style=style) + if source in ('dark', 'light', ): + label_type = '_all' + else: + label_type = '_labels_under' + style = source + (label_type if labels == 'back' + else '_nolabels') + self.url = '/'.join(s.strip('/') for s in + (stem, + '{style}/{{z}}/{{x}}/{{y}}.png'.format( + style=style) + )) elif self.source.startswith('http'): # TODO: Remove this once baselayer urls can be passed in named # map config @@ -75,16 +85,17 @@ def __init__(self, source='dark', labels='back', only_labels=False): 'moment') # self.url = source else: - raise ValueError("`source` must be one of 'dark' or 'light'") + raise ValueError("`source` must be one of 'dark', 'light', or " + "voyager") def is_basic(self): """Does BaseMap pull from CARTO default basemaps? Returns: - bool: `True` if using a CARTO basemap (Dark Matter or Positron), - `False` otherwise. + bool: `True` if using a CARTO basemap (Dark Matter, Positron or + Voyager), `False` otherwise. """ - return self.source in ('dark', 'light') + return self.source in ('dark', 'light', 'voyager', ) class QueryLayer(AbstractLayer): diff --git a/cartoframes/maps.py b/cartoframes/maps.py index b5d33ee05..701cdc1ff 100644 --- a/cartoframes/maps.py +++ b/cartoframes/maps.py @@ -19,6 +19,7 @@ def get_map_name(layers, has_zoom): num_layers = len(non_basemap_layers(layers)) has_labels = len(layers) > 1 and layers[-1].is_basemap has_time = has_time_layer(layers) + basemap_id = dict(light=0, dark=1, voyager=2)[layers[0].source] return ('cartoframes_ver{version}' '_layers{layers}' @@ -31,7 +32,7 @@ def get_map_name(layers, has_zoom): has_time=('1' if has_time else '0'), # TODO: Remove this once baselayer urls can be passed in named # map config - baseid=('1' if layers[0].source == 'dark' else '0'), + baseid=basemap_id, has_labels=('1' if has_labels else '0'), has_zoom=('1' if has_zoom else '0') ) diff --git a/test/test_context.py b/test/test_context.py index b8abcd284..a5654d01b 100644 --- a/test/test_context.py +++ b/test/test_context.py @@ -16,6 +16,7 @@ import pandas as pd WILL_SKIP = False +warnings.filterwarnings("ignore") class TestCartoContext(unittest.TestCase): @@ -533,21 +534,21 @@ def test_cartocontext_map_geom_type(self): # baseid1 = dark, labels1 = labels on top in named map name labels_polygon = cc.map(layers=Layer(self.test_read_table)) self.assertRegexpMatches(labels_polygon.__html__(), - '.*baseid1_labels1.*', + '.*baseid2_labels1.*', msg='labels should be on top since only a ' 'polygon layer is present') - # baseid1 = dark, labels0 = labels on bottom + # baseid2 = voyager, labels0 = labels on bottom labels_point = cc.map(layers=Layer(self.test_point_table)) self.assertRegexpMatches(labels_point.__html__(), - '.*baseid1_labels0.*', + '.*baseid2_labels0.*', msg='labels should be on bottom because a ' 'point layer is present') labels_multi = cc.map(layers=[Layer(self.test_point_table), Layer(self.test_read_table)]) self.assertRegexpMatches(labels_multi.__html__(), - '.*baseid1_labels0.*', + '.*baseid2_labels0.*', msg='labels should be on bottom because a ' 'point layer is present') # create a layer with points and polys, but with more polys @@ -566,7 +567,7 @@ def test_cartocontext_map_geom_type(self): points=self.test_point_table)) multi_geom = cc.map(layers=multi_geom_layer) self.assertRegexpMatches(multi_geom.__html__(), - '.*baseid1_labels1.*', + '.*baseid2_labels1.*', msg='layer has more polys than points, so it ' 'should default to polys labels (on top)') diff --git a/test/test_layer.py b/test/test_layer.py index 428c88a5b..e13f140e0 100644 --- a/test/test_layer.py +++ b/test/test_layer.py @@ -15,18 +15,23 @@ def setUp(self): # basemaps with baked-in labels self.dark_map_all = BaseMap(source='dark') self.light_map_all = BaseMap(source='light') + self.voyager_labels_under = BaseMap(source='voyager') # basemaps with no labels self.dark_map_no_labels = BaseMap(source='dark', labels=None) self.light_map_no_labels = BaseMap(source='light', labels=None) + self.voyager_map_no_labels = BaseMap(source='voyager', + labels=None) # labels with no basemaps self.dark_only_labels = BaseMap(source='dark', only_labels=True) self.light_only_labels = BaseMap(source='light', only_labels=True) + self.voyager_only_labels = BaseMap(source='voyager', + only_labels=True) def test_basemap_invalid(self): """layer.Basemap exceptions on invalid source""" @@ -53,23 +58,34 @@ def test_basemap_source(self): self.assertEqual(self.light_map_all.url, 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' 'light_all/{z}/{x}/{y}.png') + self.assertEqual(self.voyager_labels_under.url, + 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' + 'rastertiles/voyager_labels_under/{z}/{x}/{y}.png') self.assertEqual(self.dark_map_no_labels.url, 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' 'dark_nolabels/{z}/{x}/{y}.png') self.assertEqual(self.light_map_no_labels.url, 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' 'light_nolabels/{z}/{x}/{y}.png') + self.assertEqual(self.voyager_map_no_labels.url, + 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' + 'rastertiles/voyager_nolabels/{z}/{x}/{y}.png') self.assertEqual(self.light_only_labels.url, 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' 'light_only_labels/{z}/{x}/{y}.png') self.assertEqual(self.dark_only_labels.url, 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' 'dark_only_labels/{z}/{x}/{y}.png') + self.assertEqual(self.voyager_only_labels.url, + 'https://cartodb-basemaps-{s}.global.ssl.fastly.net/' + 'rastertiles/voyager_only_labels/{z}/{x}/{y}.png') # ensure self.is_basic() works as intended self.assertTrue(self.light_map_all.is_basic(), msg='is a basic carto basemap') self.assertTrue(self.dark_map_all.is_basic()) + self.assertTrue(self.voyager_labels_under.is_basic(), + msg='is a basic carto basemap') class TestQueryLayer(unittest.TestCase): From 1737675408d6c19eb9ca44109c41867f3c51fab6 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Tue, 10 Oct 2017 22:41:31 -0400 Subject: [PATCH 2/2] adds join_url for more functionalized url composition --- cartoframes/context.py | 8 ++++---- cartoframes/layer.py | 7 +++---- cartoframes/utils.py | 5 +++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cartoframes/context.py b/cartoframes/context.py index 60a6ded70..c88bee6f4 100644 --- a/cartoframes/context.py +++ b/cartoframes/context.py @@ -18,7 +18,7 @@ from carto.exceptions import CartoException from .credentials import Credentials -from .utils import dict_items, normalize_colnames, norm_colname +from .utils import dict_items, normalize_colnames, norm_colname, join_url from .layer import BaseMap from .maps import non_basemap_layers, get_map_name, get_map_template @@ -217,7 +217,7 @@ def write(self, df, table_name, temp_dir='/tmp', overwrite=False, 'minutes.\n' '\033[1mNote:\033[0m `CartoContext.map` will not work on ' 'this table until its geometries are created.'.format( - table_url='/'.join((self.creds.base_url(), + table_url=join_url((self.creds.base_url(), 'dataset', final_table_name, )), job_id=status.get('job_id'), @@ -227,7 +227,7 @@ def write(self, df, table_name, temp_dir='/tmp', overwrite=False, self.sql_client.send(query) tqdm.write('Table successfully written to CARTO: {table_url}'.format( - table_url='/'.join((self.creds.base_url(), + table_url=join_url((self.creds.base_url(), 'dataset', final_table_name, )))) @@ -734,7 +734,7 @@ def map(self, layers=None, interactive=True, options.update(self._get_bounds(nb_layers)) map_name = self._send_map_template(layers, has_zoom=has_zoom) - api_url = '/'.join((self.creds.base_url(), 'api/v1/map', )) + api_url = join_url((self.creds.base_url(), 'api/v1/map', )) static_url = ('{api_url}/static/named/{map_name}' '/{width}/{height}.png?{params}').format( diff --git a/cartoframes/layer.py b/cartoframes/layer.py index 89a7ed09f..6ae19117b 100644 --- a/cartoframes/layer.py +++ b/cartoframes/layer.py @@ -6,7 +6,7 @@ import pandas as pd import webcolors -from cartoframes.utils import cssify +from cartoframes.utils import cssify, join_url from cartoframes.styling import BinMethod, mint, antique, get_scheme_cartocss # colors map data layers without color specified @@ -73,8 +73,7 @@ def __init__(self, source='voyager', labels='back', only_labels=False): label_type = '_labels_under' style = source + (label_type if labels == 'back' else '_nolabels') - self.url = '/'.join(s.strip('/') for s in - (stem, + self.url = join_url((stem, '{style}/{{z}}/{{x}}/{{y}}.png'.format( style=style) )) @@ -86,7 +85,7 @@ def __init__(self, source='voyager', labels='back', only_labels=False): # self.url = source else: raise ValueError("`source` must be one of 'dark', 'light', or " - "voyager") + "'voyager'") def is_basic(self): """Does BaseMap pull from CARTO default basemaps? diff --git a/cartoframes/utils.py b/cartoframes/utils.py index ae5750e40..5f8b95f34 100644 --- a/cartoframes/utils.py +++ b/cartoframes/utils.py @@ -73,3 +73,8 @@ def norm_colname(colname): if final_name[0].isdigit(): return '_' + final_name return final_name + + +def join_url(parts): + """join parts of URL into complete url""" + return '/'.join(s.strip('/') for s in parts)