Skip to content
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

Specs cleanup #477

Merged
merged 4 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
Cleanup only whitelisted lists
  • Loading branch information
IKupriyanov-HORIS committed Dec 15, 2021
commit 5c0a56ca0340de3c6070d8349102fdcb94967d26
60 changes: 33 additions & 27 deletions python-package/lets_plot/plot/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,37 +130,43 @@ def layer(geom=None, stat=None, data=None, mapping=None, position=None, **kwargs
return LayerSpec(**locals())


def _cleanup(obj):
# Empty corr_plot layer adds geom with default visuals, if remove - no layer will be added at all
keep_if_empty = ['point_params', 'tile_params', 'label_params']
def _cleanup_spec_list(obj):
assert isinstance(obj, list)

if isinstance(obj, dict):
res = {}
for k, v in obj.items():
if v is None:
continue
if all(isinstance(item, (dict, type(None))) for item in obj):
return [v for v in map(lambda item: _cleanup_spec(item), obj) if v is not None]

if k in ['data', 'map']:
res[k] = v
continue
# ignore non-dict list or it may produce unexpected result: [['asd'], [None]] => [['asd'],[]]
return obj


def _cleanup_spec(obj: dict):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks tricky, has too much knowledge about internals, trying to do too much when this is not required.

Lets's use _filter_none (move it to core, may be rename) in _specs_to_dict(opts_raw) when v is a dict.

assert isinstance(obj, dict)
res = {}
for k, v in obj.items():
if v is None:
continue

if k in ['data', 'map']:
res[k] = v
continue

if isinstance(v, (dict, list)):
clean = _cleanup(v)
if len(clean) > 0 or k in keep_if_empty:
if isinstance(v, list):
if k in ['scales', 'layers', 'feature-list', 'tooltip_formats', 'tooltip_lines', 'tooltip_variables']:
clean = _cleanup_spec_list(v)
if len(clean) > 0:
res[k] = clean
else:
res[k] = v
return res

if isinstance(obj, list):
if all(isinstance(item, (dict, type(None))) for item in obj):
return [v for v in map(lambda item: _cleanup(item), obj) if v is not None]

# ignore non-dict list or it may produce unexpected result: [['asd'], [None]] => [['asd'],[]]
return obj

else:
return obj
elif isinstance(v, dict):
clean = _cleanup_spec(v)
# Empty corr_plot layer adds geom with default visuals, if remove - no layer will be added at all
if len(clean) > 0 or k in ['point_params', 'tile_params', 'label_params']:
res[k] = clean
else:
res[k] = v
return res


#
Expand All @@ -178,7 +184,7 @@ def _specs_to_dict(opts_raw):
else:
opts[k] = v

return _cleanup(opts)
return _cleanup_spec(opts)


class FeatureSpec():
Expand Down Expand Up @@ -420,7 +426,7 @@ def as_dict(self):
d['scales'] = [scale.as_dict() for scale in self.__scales]
d['layers'] = [layer.as_dict() for layer in self.__layers]

return _cleanup(d)
return _cleanup_spec(d)

def __str__(self):
result = ['plot']
Expand Down Expand Up @@ -561,7 +567,7 @@ def elements(self):

def as_dict(self):
elements = [{e.kind: e.as_dict()} for e in self.__elements]
return _cleanup({'feature-list': elements})
return _cleanup_spec({'feature-list': elements})

def __add__(self, other):
if isinstance(other, DummySpec):
Expand Down
4 changes: 2 additions & 2 deletions python-package/lets_plot/plot/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from lets_plot._global_settings import has_global_value, get_global_val, MAX_WIDTH, MAX_HEIGHT
from lets_plot.geo_data_internals.utils import is_geocoder
from lets_plot.plot.core import FeatureSpec, _cleanup
from lets_plot.plot.core import FeatureSpec, _cleanup_spec
from lets_plot.plot.core import PlotSpec
from lets_plot.plot.util import as_annotated_data

Expand Down Expand Up @@ -211,7 +211,7 @@ def item_as_dict(item):
return result

d['items'] = [item_as_dict(item) for item in self.items]
return _cleanup(d)
return _cleanup_spec(d)

def _repr_html_(self):
"""
Expand Down
4 changes: 2 additions & 2 deletions python-package/lets_plot/plot/tooltip.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from typing import List

from lets_plot.plot.core import FeatureSpec, _cleanup
from lets_plot.plot.core import FeatureSpec, _cleanup_spec

#
# Tooltips
Expand Down Expand Up @@ -116,7 +116,7 @@ def as_dict(self):
d['tooltip_min_width'] = self._tooltip_min_width
d['tooltip_color'] = self._tooltip_color
d['tooltip_variables'] = self._tooltip_variables
return _cleanup(d)
return _cleanup_spec(d)

def format(self, field=None, format=None):
"""
Expand Down
18 changes: 18 additions & 0 deletions python-package/test/plot/test_geom_livemap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright (c) 2021. JetBrains s.r.o.
# Use of this source code is governed by the MIT license that can be found in the LICENSE file.
import pytest

import lets_plot as gg


@pytest.mark.parametrize('args,expected', [
('foo', [['foo'], None]),
(['foo'], [['foo'], None]),
(['foo', 'bar'], [['foo'], ['bar']]),
([['foo', 'bar']], [['foo', 'bar'], None]),
([['foo', 'bar'], ['baz', 'qux']], [['foo', 'bar'], ['baz', 'qux']])
])
def test_map_join(args, expected):
# ggplot is required - it normalizes map_join on before_append
spec = gg.ggplot() + gg.geom_livemap(map_join=args, tooltips=gg.layer_tooltips())
assert spec.as_dict()['layers'][0]['map_join'] == expected