Skip to content

Commit 63fdcc3

Browse files
authored
Merge pull request #47 from jmishra01/few-changes
Code refactoring
2 parents cc197bf + 4ccb17c commit 63fdcc3

File tree

3 files changed

+69
-86
lines changed

3 files changed

+69
-86
lines changed

highcharts_core/chart.py

Lines changed: 60 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,16 @@ def _jupyter_include_scripts(self):
4343
4444
:rtype: :class:`str <python:str>`
4545
"""
46-
js_str = ''
47-
for item in constants.INCLUDE_LIBS:
48-
js_str += utility_functions.jupyter_add_script(item)
49-
js_str += """.then(() => {"""
50-
51-
for item in constants.INCLUDE_LIBS:
52-
js_str += """});"""
46+
js_str = ''.join(
47+
utility_functions.jupyter_add_script(item) + """.then(() => {"""
48+
for item in constants.INCLUDE_LIBS
49+
)
50+
js_str += """});""" * len(constants.INCLUDE_LIBS)
5351

5452
return js_str
5553

56-
def _jupyter_javascript(self,
57-
global_options = None,
54+
def _jupyter_javascript(self,
55+
global_options = None,
5856
container = None,
5957
random_slug = None,
6058
retries = 3,
@@ -65,21 +63,21 @@ def _jupyter_javascript(self,
6563
Defaults to :obj:`None <python:None>`
6664
:type global_options: :class:`SharedOptions <highcharts_stock.global_options.shared_options.SharedOptions>`
6765
or :obj:`None <python:None>`
68-
66+
6967
:param container: The ID to apply to the HTML container when rendered in Jupyter Labs. Defaults to
70-
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
68+
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
7169
property if set, and ``'highcharts_target_div'`` if not set.
7270
:type container: :class:`str <python:str>` or :obj:`None <python:None>`
73-
71+
7472
:param random_slug: The random sequence of characters to append to the container name to ensure uniqueness.
7573
Defaults to :obj:`None <python:None>`
7674
:type random_slug: :class:`str <python:str>` or :obj:`None <python:None>`
77-
78-
:param retries: The number of times to retry rendering the chart. Used to avoid race conditions with the
75+
76+
:param retries: The number of times to retry rendering the chart. Used to avoid race conditions with the
7977
Highcharts script. Defaults to 3.
8078
:type retries: :class:`int <python:int>`
81-
82-
:param interval: The number of milliseconds to wait between retrying rendering the chart. Defaults to 1000 (1
79+
80+
:param interval: The number of milliseconds to wait between retrying rendering the chart. Defaults to 1000 (1
8381
seocnd).
8482
:type interval: :class:`int <python:int>`
8583
@@ -91,14 +89,13 @@ def _jupyter_javascript(self,
9189
self.container = new_container
9290
else:
9391
self.container = f'{new_container}_{random_slug}'
94-
92+
9593
if global_options is not None:
9694
global_options = validate_types(global_options,
9795
types = SharedOptions)
9896

99-
js_str = ''
100-
js_str += utility_functions.get_retryHighcharts()
101-
97+
js_str = utility_functions.get_retryHighcharts()
98+
10299
if global_options:
103100
js_str += '\n' + utility_functions.prep_js_for_jupyter(global_options.to_js_literal()) + '\n'
104101

@@ -118,11 +115,11 @@ def _jupyter_container_html(self,
118115
"""Returns the Jupyter Labs HTML container for rendering the chart in Jupyter Labs context.
119116
120117
:param container: The ID to apply to the HTML container when rendered in Jupyter Labs. Defaults to
121-
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
118+
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
122119
property if set, and ``'highcharts_target_div'`` if not set.
123120
:type container: :class:`str <python:str>` or :obj:`None <python:None>`
124121
125-
:param random_slug: The random sequence of characters to append to the container/function name to ensure
122+
:param random_slug: The random sequence of characters to append to the container/function name to ensure
126123
uniqueness. Defaults to :obj:`None <python:None>`
127124
:type random_slug: :class:`str <python:str>` or :obj:`None <python:None>`
128125
@@ -296,8 +293,6 @@ def to_js_literal(self,
296293
297294
:rtype: :class:`str <python:str>` or :obj:`None <python:None>`
298295
"""
299-
if filename:
300-
filename = validators.path(filename)
301296

302297
untrimmed = self._to_untrimmed_dict()
303298
as_dict = {}
@@ -307,27 +302,21 @@ def to_js_literal(self,
307302
if serialized is not None:
308303
as_dict[key] = serialized
309304

310-
signature_elements = 0
305+
signature_elements = 2
311306

312-
container_as_str = ''
313307
if self.container:
314308
container_as_str = f"""'{self.container}'"""
315309
else:
316310
container_as_str = """null"""
317-
signature_elements += 1
318311

319-
options_as_str = ''
320312
if self.options:
321-
options_as_str = self.options.to_js_literal(encoding = encoding)
322-
options_as_str = f"""{options_as_str}"""
313+
options_as_str = "{}".format(self.options.to_js_literal(encoding = encoding))
323314
else:
324315
options_as_str = """null"""
325-
signature_elements += 1
326316

327317
callback_as_str = ''
328318
if self.callback:
329-
callback_as_str = self.callback.to_js_literal(encoding = encoding)
330-
callback_as_str = f"""{callback_as_str}"""
319+
callback_as_str = "{}".format(self.callback.to_js_literal(encoding = encoding))
331320
signature_elements += 1
332321

333322
signature = """Highcharts.chart("""
@@ -351,7 +340,7 @@ def to_js_literal(self,
351340
suffix = """});"""
352341
as_str = prefix + as_str + '\n' + suffix
353342

354-
if filename:
343+
if validators.path(filename, allow_empty = True):
355344
with open(filename, 'w', encoding = encoding) as file_:
356345
file_.write(as_str)
357346

@@ -463,14 +452,14 @@ def _copy_dict_key(cls,
463452

464453
if key == 'data' and preserve_data:
465454
return other_value
466-
455+
467456
if key == 'points' and preserve_data:
468457
return other_value
469-
458+
470459
if key == 'series' and preserve_data:
471460
if not other_value:
472461
return [x for x in original_value]
473-
462+
474463
if len(other_value) != len(original_value):
475464
matched_series = []
476465
new_series = []
@@ -501,14 +490,12 @@ def _copy_dict_key(cls,
501490
return updated_series
502491

503492
elif isinstance(original_value, (dict, UserDict)):
504-
new_value = {}
505-
for subkey in original_value:
506-
new_key_value = cls._copy_dict_key(subkey,
493+
new_value = {subkey: cls._copy_dict_key(subkey,
507494
original_value,
508495
other_value,
509496
overwrite = overwrite,
510497
**kwargs)
511-
new_value[subkey] = new_key_value
498+
for subkey in original_value}
512499

513500
return new_value
514501

@@ -576,10 +563,8 @@ def add_series(self, *series):
576563
or coercable
577564
578565
"""
579-
new_series = []
580-
for item in series:
581-
item_series = create_series_obj(item)
582-
new_series.append(item_series)
566+
new_series = [create_series_obj(item)
567+
for item in series]
583568

584569
if self.options and self.options.series:
585570
existing_series = [x for x in self.options.series]
@@ -594,8 +579,8 @@ def add_series(self, *series):
594579
self.options.series = updated_series
595580

596581
def update_series(self, *series, add_if_unmatched = False):
597-
"""Replace existing series with the new versions supplied in ``series``,
598-
matching them based on their
582+
"""Replace existing series with the new versions supplied in ``series``,
583+
matching them based on their
599584
:meth:`.id <highcharts_core.options.series.base.SeriesBase.id>` property.
600585
601586
:param series: One or more :term:`series` instances (descended from
@@ -605,17 +590,15 @@ def update_series(self, *series, add_if_unmatched = False):
605590
:type series: one or more
606591
:class:`SeriesBase <highcharts_core.options.series.base.SeriesBase>`
607592
or coercable
608-
609-
:param add_if_unmatched: If ``True``, will add a series that does not have a
610-
match. If ``False``, will raise a
593+
594+
:param add_if_unmatched: If ``True``, will add a series that does not have a
595+
match. If ``False``, will raise a
611596
:exc:`HighchartsMissingSeriesError <highcharts_core.errors.HighchartsMissingSeriesError>`
612597
if a series does not have a match on the chart. Defaults to ``False``.
613598
:type add_if_unmatched: :class:`bool <python:bool>`
614599
"""
615-
new_series = []
616-
for item in series:
617-
item_series = create_series_obj(item)
618-
new_series.append(item_series)
600+
new_series = [create_series_obj(item)
601+
for item in series]
619602

620603
if self.options and self.options.series:
621604
existing_series = [x for x in self.options.series]
@@ -624,23 +607,22 @@ def update_series(self, *series, add_if_unmatched = False):
624607
else:
625608
existing_series = []
626609
self.options = HighchartsOptions()
627-
610+
628611
existing_ids = [x.id for x in existing_series]
629612
new_ids = [x.id for x in new_series]
630613
overlap_ids = [x for x in new_ids if x in existing_ids]
631-
632-
updated_series = []
633-
for existing in existing_series:
634-
if existing.id not in overlap_ids:
635-
updated_series.append(existing)
636-
614+
615+
updated_series = [existing
616+
for existing in existing_series
617+
if existing.id not in overlap_ids]
618+
637619
for new in new_series:
638620
if new.id not in overlap_ids and not add_if_unmatched:
639621
raise errors.HighchartsMissingSeriesError(f'attempted to update series '
640622
f'id "{new.id}", but that '
641623
f'series is not present in '
642624
f'the chart')
643-
625+
644626
updated_series.append(new)
645627

646628
self.options.series = updated_series
@@ -680,11 +662,11 @@ def from_series(cls, *series, kwargs = None):
680662
instance.add_series(item)
681663
else:
682664
instance.add_series(series)
683-
665+
684666
return instance
685667

686-
def display(self,
687-
global_options = None,
668+
def display(self,
669+
global_options = None,
688670
container = None,
689671
retries = 3,
690672
interval = 1000):
@@ -695,29 +677,29 @@ def display(self,
695677
Defaults to :obj:`None <python:None>`
696678
:type global_options: :class:`SharedOptions <highcharts_stock.global_options.shared_options.SharedOptions>`
697679
or :obj:`None <python:None>`
698-
680+
699681
:param container: The ID to apply to the HTML container when rendered in Jupyter Labs. Defaults to
700-
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
682+
:obj:`None <python:None>`, which applies the :meth:`.container <highcharts_core.chart.Chart.container>`
701683
property if set, and ``'highcharts_target_div'`` if not set.
702-
684+
703685
.. note::
704-
686+
705687
Highcharts for Python will append a 6-character random string to the value of ``container``
706688
to ensure uniqueness of the chart's container when rendering in a Jupyter Notebook/Labs context. The
707-
:class:`Chart <highcharts_core.chart.Chart>` instance will retain the mapping between container and the
689+
:class:`Chart <highcharts_core.chart.Chart>` instance will retain the mapping between container and the
708690
random string so long as the instance exists, thus allowing you to easily update the rendered chart by
709691
calling the :meth:`.display() <highcharts_core.chart.Chart.display>` method again.
710-
692+
711693
If you wish to create a new chart from the instance that does not update the existing chart, then you can do
712694
so by specifying a new ``container`` value.
713-
695+
714696
:type container: :class:`str <python:str>` or :obj:`None <python:None>`
715697
716-
:param retries: The number of times to retry rendering the chart. Used to avoid race conditions with the
698+
:param retries: The number of times to retry rendering the chart. Used to avoid race conditions with the
717699
Highcharts script. Defaults to 3.
718700
:type retries: :class:`int <python:int>`
719-
720-
:param interval: The number of milliseconds to wait between retrying rendering the chart. Defaults to 1000 (1
701+
702+
:param interval: The number of milliseconds to wait between retrying rendering the chart. Defaults to 1000 (1
721703
seocnd).
722704
:type interval: :class:`int <python:int>`
723705
@@ -740,17 +722,17 @@ def display(self,
740722
container = container or self.container or 'highcharts_target_div'
741723
if not self._random_slug:
742724
self._random_slug = {}
743-
725+
744726
random_slug = self._random_slug.get(container, None)
745-
727+
746728
if not random_slug:
747729
random_slug = utility_functions.get_random_string()
748730
self._random_slug[container] = random_slug
749731

750732
html_str = self._jupyter_container_html(container, random_slug)
751733
html_display = display_mod.HTML(data = html_str)
752734

753-
chart_js_str = self._jupyter_javascript(global_options = global_options,
735+
chart_js_str = self._jupyter_javascript(global_options = global_options,
754736
container = container,
755737
random_slug = random_slug,
756738
retries = retries,

highcharts_core/global_options/shared_options.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import esprima
44
from esprima.error_handler import Error as ParseError
55

6+
from validator_collection import validators, checkers
7+
68
from highcharts_core import errors
79
from highcharts_core.decorators import validate_types
810
from highcharts_core.options import HighchartsOptions
@@ -42,7 +44,7 @@ def to_js_literal(self,
4244

4345
as_str = prefix + options_body + ');'
4446

45-
if filename:
47+
if validators.path(filename, allow_empty = True):
4648
with open(filename, 'w', encoding = encoding) as file_:
4749
file_.write(as_str)
4850

highcharts_core/metaclasses.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,12 @@ def from_dict(cls,
197197
"""
198198
as_dict = validators.dict(as_dict, allow_empty = True) or {}
199199
clean_as_dict = {}
200-
for key in as_dict:
201-
if allow_snake_case:
202-
clean_key = utility_functions.to_camelCase(key)
203-
else:
204-
clean_key = key
205-
206-
clean_as_dict[clean_key] = as_dict[key]
200+
if allow_snake_case:
201+
clean_as_dict = {utility_functions.to_camelCase(key): as_dict[key]
202+
for key in as_dict}
203+
else:
204+
clean_as_dict = {key: as_dict[key]
205+
for key in as_dict}
207206

208207
kwargs = cls._get_kwargs_from_dict(clean_as_dict)
209208

0 commit comments

Comments
 (0)