From 1313e201691dcb9ca861fe73d9a4aff021b2406e Mon Sep 17 00:00:00 2001 From: rocky Date: Wed, 25 Dec 2024 05:59:28 -0500 Subject: [PATCH 1/4] Add "lpn" checking in _ListPlot "lpn" is "list of points" checking on ListPlot[] and ListLinePlot[] --- mathics/builtin/drawing/plot.py | 17 +++++++++++++- mathics/eval/drawing/plot.py | 2 +- test/builtin/drawing/test_plot.py | 37 ++++++++++++++++++++++++++----- test/helper.py | 2 +- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/mathics/builtin/drawing/plot.py b/mathics/builtin/drawing/plot.py index 9387c3534..e9eef5951 100644 --- a/mathics/builtin/drawing/plot.py +++ b/mathics/builtin/drawing/plot.py @@ -286,11 +286,12 @@ class _ListPlot(Builtin, ABC): attributes = A_PROTECTED | A_READ_PROTECTED messages = { + "joind": "Value of option Joined -> `1` is not True or False.", + "lpn": "`1` is not a list of numbers or pairs of numbers.", "prng": ( "Value of option PlotRange -> `1` is not All, Automatic or " "an appropriate list of range specifications." ), - "joind": "Value of option Joined -> `1` is not True or False.", } use_log_scale = False @@ -300,6 +301,20 @@ def eval(self, points, evaluation: Evaluation, options: dict): class_name = self.__class__.__name__ + if not isinstance(points, ListExpression): + evaluation.message(class_name, "lpn", points) + return + + if not all( + element.is_numeric(evaluation) + or isinstance(element, ListExpression) + or (1 <= len(element.elements) <= 2) + or (len(element.elements) == 1 and isinstance(element[0], ListExpression)) + for element in points.elements + ): + evaluation.message(class_name, "lpn", points) + return + # Scale point values down by Log 10. Tick mark values will be adjusted to be 10^n in GraphicsBox. if self.use_log_scale: points = ListExpression( diff --git a/mathics/eval/drawing/plot.py b/mathics/eval/drawing/plot.py index 3d34424c7..7a0e51748 100644 --- a/mathics/eval/drawing/plot.py +++ b/mathics/eval/drawing/plot.py @@ -229,7 +229,7 @@ def eval_ListPlot( # Split into plot segments plot_groups = [[plot_group] for plot_group in plot_groups] if isinstance(x_range, (list, tuple)): - x_min, m_max = x_range + x_min, x_max = x_range y_min, y_max = y_range for lidx, plot_group in enumerate(plot_groups): diff --git a/test/builtin/drawing/test_plot.py b/test/builtin/drawing/test_plot.py index 670ddbaf4..0cc3ad585 100644 --- a/test/builtin/drawing/test_plot.py +++ b/test/builtin/drawing/test_plot.py @@ -3,13 +3,40 @@ Unit tests from mathics.builtin.drawing.plot """ -import sys -import time -from test.helper import check_evaluation, evaluate +from test.helper import check_evaluation import pytest +def test__listplot(): + """tests for module builtin.drawing.plot._ListPlot""" + for str_expr, msgs, str_expected, fail_msg in ( + ( + "ListPlot[5]", + ("5 is not a list of numbers or pairs of numbers.",), + "ListPlot[5]", + "ListPlot with invalid list of point", + ), + ( + "ListLinePlot[{{}, {{1., 1.}}, {{1., 2.}}, {}}]", + ( + "{{}, {{1., 1.}}, {{1., 2.}}, {}} is not a list of numbers or pairs of numbers.", + ), + "ListLinePlot[{{}, {{1., 1.}}, {{1., 2.}}, {}}]", + "ListLinePlot with invalid list of point", + ), + ): + check_evaluation( + str_expr, + str_expected, + to_string_expr=True, + to_string_expected=True, + hold_expected=True, + failure_message=fail_msg, + expected_messages=msgs, + ) + + @pytest.mark.parametrize( ("str_expr", "msgs", "str_expected", "fail_msg"), [ @@ -159,8 +186,8 @@ ), ], ) -def test_private_doctests_plot(str_expr, msgs, str_expected, fail_msg): - """builtin.drawing.plot""" +def test_plot(str_expr, msgs, str_expected, fail_msg): + """tests for module builtin.drawing.plot""" check_evaluation( str_expr, str_expected, diff --git a/test/helper.py b/test/helper.py index 58ebc17f6..6e13f7982 100644 --- a/test/helper.py +++ b/test/helper.py @@ -38,7 +38,7 @@ def evaluate(str_expr: str): def check_evaluation( str_expr: Optional[str], str_expected: Optional[str] = None, - failure_message: str = "", + failure_message: Optional[str] = "", hold_expected: bool = False, to_string_expr: Optional[bool] = True, to_string_expected: bool = True, From 96eb94ef139ecc51fb5dd439dfa672b6c81a99ff Mon Sep 17 00:00:00 2001 From: rocky Date: Wed, 25 Dec 2024 08:21:18 -0500 Subject: [PATCH 2/4] Weird changes needed... _ListPlot[] needs to evaluate its arguements in order to check them! There is this weird place in caching where we were caching the Expression form of a ListExpression. This is something that has been plaguing our code. Finding and fixing these simplifies coding speeds up execution. --- mathics/builtin/drawing/plot.py | 19 ++++++++++--------- mathics/core/expression.py | 8 ++++++-- mathics/eval/drawing/plot.py | 8 ++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/mathics/builtin/drawing/plot.py b/mathics/builtin/drawing/plot.py index e9eef5951..50ae1f26f 100644 --- a/mathics/builtin/drawing/plot.py +++ b/mathics/builtin/drawing/plot.py @@ -301,6 +301,16 @@ def eval(self, points, evaluation: Evaluation, options: dict): class_name = self.__class__.__name__ + # Scale point values down by Log 10. Tick mark values will be adjusted to be 10^n in GraphicsBox. + if self.use_log_scale: + points = ListExpression( + *( + Expression(SymbolLog10, point).evaluate(evaluation) + for point in points + ) + ) + + points = points.evaluate(evaluation) if not isinstance(points, ListExpression): evaluation.message(class_name, "lpn", points) return @@ -315,15 +325,6 @@ def eval(self, points, evaluation: Evaluation, options: dict): evaluation.message(class_name, "lpn", points) return - # Scale point values down by Log 10. Tick mark values will be adjusted to be 10^n in GraphicsBox. - if self.use_log_scale: - points = ListExpression( - *( - Expression(SymbolLog10, point).evaluate(evaluation) - for point in points - ) - ) - # If "points" is a literal value with a Python representation, # it has a ".value" attribute with a non-None value. So here, # we don't have to eval_N().to_python(). diff --git a/mathics/core/expression.py b/mathics/core/expression.py index 927791f70..b96085e9f 100644 --- a/mathics/core/expression.py +++ b/mathics/core/expression.py @@ -1410,8 +1410,12 @@ def rules(): if not isinstance(result, EvalMixin): return result, False if result.sameQ(new): - new._timestamp_cache(evaluation) - return new, False + # Even though result and new may be the same, + # new can be a Expression[SymbolConstant: System`List...] + # while "result' might be ListExpression!? + # So make sure to use "result", not "new". + result._timestamp_cache(evaluation) + return result, False else: return result, True diff --git a/mathics/eval/drawing/plot.py b/mathics/eval/drawing/plot.py index 7a0e51748..addcf2b47 100644 --- a/mathics/eval/drawing/plot.py +++ b/mathics/eval/drawing/plot.py @@ -201,14 +201,14 @@ def eval_ListPlot( [xx for xx, yy in plot_groups], [xx for xx, yy in plot_groups], x_range ) plot_groups = [plot_groups] - elif all(isinstance(line, list) for line in plot_groups): - if not all(isinstance(line, list) for line in plot_groups): + elif all(isinstance(line, (list, tuple)) for line in plot_groups): + if not all(isinstance(line, (list, tuple)) for line in plot_groups): return # He have a list of plot groups if all( - isinstance(point, list) and len(point) == 2 - for plot_group in plot_groups + isinstance(point, (list, tuple)) and len(point) == 2 + for _ in plot_groups for point in plot_groups ): pass From 0b8d56649736bae6e4a640565c10b89486883a05 Mon Sep 17 00:00:00 2001 From: rocky Date: Wed, 25 Dec 2024 08:31:21 -0500 Subject: [PATCH 3/4] Add check for mypy --- mathics/core/expression.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mathics/core/expression.py b/mathics/core/expression.py index b96085e9f..83e37bab4 100644 --- a/mathics/core/expression.py +++ b/mathics/core/expression.py @@ -1414,7 +1414,8 @@ def rules(): # new can be a Expression[SymbolConstant: System`List...] # while "result' might be ListExpression!? # So make sure to use "result", not "new". - result._timestamp_cache(evaluation) + if isinstance(result, Expression): + result._timestamp_cache(evaluation) return result, False else: return result, True From c6337b72e28310124a115ef5cbc901ab8b812ade Mon Sep 17 00:00:00 2001 From: rocky Date: Wed, 25 Dec 2024 08:36:24 -0500 Subject: [PATCH 4/4] NO GOOD: allow tuple in another place --- mathics/eval/drawing/plot.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mathics/eval/drawing/plot.py b/mathics/eval/drawing/plot.py index addcf2b47..c89a91d14 100644 --- a/mathics/eval/drawing/plot.py +++ b/mathics/eval/drawing/plot.py @@ -213,7 +213,9 @@ def eval_ListPlot( ): pass elif not is_discrete_plot and all( - not isinstance(point, list) for line in plot_groups for point in line + not isinstance(point, (list, tuple)) + for line in plot_groups + for point in line ): # FIXME: is this right? y_min = min(plot_groups)[0]