From 58ed3e88b8c2f6ea451a11fff26d71b2881c43ce Mon Sep 17 00:00:00 2001 From: rocky Date: Wed, 25 Sep 2024 19:45:38 -0400 Subject: [PATCH 1/5] tweak some error messages to be more like WMA... in particular Get::noopen, {First,Last,Most}::normal --- mathics/builtin/list/eol.py | 35 +++++++++++++++++++--------------- mathics/eval/files_io/files.py | 2 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/mathics/builtin/list/eol.py b/mathics/builtin/list/eol.py index 21eb8072c..416b8b1de 100644 --- a/mathics/builtin/list/eol.py +++ b/mathics/builtin/list/eol.py @@ -21,6 +21,7 @@ from mathics.core.builtin import BinaryOperator, Builtin from mathics.core.convert.expression import to_mathics_list from mathics.core.convert.python import from_python +from mathics.core.evaluation import Evaluation from mathics.core.exceptions import ( InvalidLevelspecError, MessageException, @@ -589,7 +590,7 @@ class First(Builtin): >> First[a + b + c] = a >> First[x] - : Nonatomic expression expected. + : Nonatomic expression expected at position 1 in First[x]. = First[x] >> First[{}] : {} has zero length and no first element. @@ -597,16 +598,16 @@ class First(Builtin): """ messages = { - "normal": "Nonatomic expression expected.", + "normal": "Nonatomic expression expected at position 1 in `1`.", "nofirst": "`1` has zero length and no first element.", } summary_text = "first element of a list or expression" - def eval(self, expr, evaluation): + def eval(self, expr, evaluation: Evaluation, expression: Expression): "First[expr_]" if isinstance(expr, Atom): - evaluation.message("First", "normal") + evaluation.message("First", "normal", expression) return if len(expr.elements) == 0: evaluation.message("First", "nofirst", expr) @@ -820,7 +821,7 @@ class Last(Builtin): >> Last[{a, b, c}] = c >> Last[x] - : Nonatomic expression expected. + : Nonatomic expression expected at position 1 in Last[x]. = Last[x] >> Last[{}] : {} has zero length and no last element. @@ -828,16 +829,16 @@ class Last(Builtin): """ messages = { - "normal": "Nonatomic expression expected.", + "normal": "Nonatomic expression expected at position 1 in `1`.", "nolast": "`1` has zero length and no last element.", } summary_text = "last element of a list or expression" - def eval(self, expr, evaluation): + def eval(self, expr, evaluation: Evaluation, expression: Expression): "Last[expr_]" if isinstance(expr, Atom): - evaluation.message("Last", "normal") + evaluation.message("Last", "normal", expression) return if len(expr.elements) == 0: evaluation.message("Last", "nolast", expr) @@ -906,17 +907,21 @@ class Most(Builtin): >> Most[a + b + c] = a + b >> Most[x] - : Nonatomic expression expected. + : Nonatomic expression expected at position 1 in Most[x]. = Most[x] """ + messages = { + "normal": "Nonatomic expression expected at position 1 in `1`.", + } + summary_text = "remove the last element" - def eval(self, expr, evaluation): + def eval(self, expr, evaluation: Evaluation, expression: Expression): "Most[expr_]" if isinstance(expr, Atom): - evaluation.message("Most", "normal") + evaluation.message("Most", "normal", expression) return return expr.slice(expr.head, slice(0, -1), evaluation) @@ -1395,7 +1400,7 @@ class Rest(Builtin): >> Rest[a + b + c] = b + c >> Rest[x] - : Nonatomic expression expected. + : Nonatomic expression expected at position 1 in Rest[x]. = Rest[x] >> Rest[{}] : Cannot take Rest of expression {} with length zero. @@ -1403,16 +1408,16 @@ class Rest(Builtin): """ messages = { - "normal": "Nonatomic expression expected.", + "normal": "Nonatomic expression expected at position 1 in `1`.", "norest": "Cannot take Rest of expression `1` with length zero.", } summary_text = "remove the first element" - def eval(self, expr, evaluation): + def eval(self, expr, evaluation: Evaluation, expression: Expression): "Rest[expr_]" if isinstance(expr, Atom): - evaluation.message("Rest", "normal") + evaluation.message("Rest", "normal", expression) return if len(expr.elements) == 0: evaluation.message("Rest", "norest", expr) diff --git a/mathics/eval/files_io/files.py b/mathics/eval/files_io/files.py index 3678f6de1..a47b0c406 100644 --- a/mathics/eval/files_io/files.py +++ b/mathics/eval/files_io/files.py @@ -73,7 +73,7 @@ def eval_Get(path: str, evaluation: Evaluation, trace_fn: Optional[Callable]): continue result = query.evaluate(evaluation) except IOError: - evaluation.message("General", "noopen", path) + evaluation.message("Get", "noopen", path) return SymbolFailed except MessageException as e: e.message(evaluation) From 7667677f504d181837f6b6ab3c9f58ce23ec2e76 Mon Sep 17 00:00:00 2001 From: rocky Date: Thu, 26 Sep 2024 07:21:01 -0400 Subject: [PATCH 2/5] Address review observations... Properties on First and Last should be A_HOLD_REST (axel) Check that arguments are 1 or 2 (mmatera) --- mathics/builtin/list/eol.py | 8 +++++++- test/builtin/list/__init__.py | 0 test/builtin/list/test_eol.py | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 test/builtin/list/__init__.py diff --git a/mathics/builtin/list/eol.py b/mathics/builtin/list/eol.py index 416b8b1de..ff3c7fb93 100644 --- a/mathics/builtin/list/eol.py +++ b/mathics/builtin/list/eol.py @@ -597,14 +597,16 @@ class First(Builtin): = First[{}] """ + attributes = A_HOLD_REST | A_PROTECTED messages = { + "argt": "First called with `1` arguments; 1 or 2 arguments are expected.", "normal": "Nonatomic expression expected at position 1 in `1`.", "nofirst": "`1` has zero length and no first element.", } summary_text = "first element of a list or expression" def eval(self, expr, evaluation: Evaluation, expression: Expression): - "First[expr_]" + "First[expr__]" if isinstance(expr, Atom): evaluation.message("First", "normal", expression) @@ -612,6 +614,9 @@ def eval(self, expr, evaluation: Evaluation, expression: Expression): if len(expr.elements) == 0: evaluation.message("First", "nofirst", expr) return + if len(expr.elements) > 2 and expr.head is SymbolSequence: + evaluation.message("First", "argt", len(expr.elements)) + return return expr.elements[0] @@ -828,6 +833,7 @@ class Last(Builtin): = Last[{}] """ + attributes = A_HOLD_REST | A_PROTECTED messages = { "normal": "Nonatomic expression expected at position 1 in `1`.", "nolast": "`1` has zero length and no last element.", diff --git a/test/builtin/list/__init__.py b/test/builtin/list/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index a0f223998..16d7c9f91 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -82,6 +82,12 @@ "Drop[{1, 2, 3, 4, 5, 6}, {-5, -2, -2}]", None, ), + ( + "First[a, b, c]", + ("First called with 3 arguments; 1 or 2 arguments are expected.",), + "First[a, b, c]", + None, + ), ('FirstPosition[{1, 2, 3}, _?StringQ, "NoStrings"]', None, "NoStrings", None), ("FirstPosition[a, a]", None, "{}", None), ( From 73b99f0fab0b5a6014e2e217f2fd94b31cacf93c Mon Sep 17 00:00:00 2001 From: rocky Date: Thu, 26 Sep 2024 07:57:39 -0400 Subject: [PATCH 3/5] Handle default args on First and Last --- mathics/builtin/list/eol.py | 49 +++++++++++++++++++++++++++++++---- test/builtin/list/test_eol.py | 8 +++++- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/mathics/builtin/list/eol.py b/mathics/builtin/list/eol.py index ff3c7fb93..097cf41dc 100644 --- a/mathics/builtin/list/eol.py +++ b/mathics/builtin/list/eol.py @@ -581,20 +581,32 @@ class First(Builtin):
'First[$expr$]'
returns the first element in $expr$. + +
'First[$expr$, $def$]' +
returns the first element in $expr$ if it exists or $def$ otherwise.
'First[$expr$]' is equivalent to '$expr$[[1]]'. >> First[{a, b, c}] = a + + The first argument need not be a list: >> First[a + b + c] = a + >> First[x] : Nonatomic expression expected at position 1 in First[x]. = First[x] + >> First[{}] : {} has zero length and no first element. = First[{}] + + As before, the first argument is empty, but a default argument is given, \ + so evaluate and return the second argument: + >> First[{}, 1+2] + = 3 """ attributes = A_HOLD_REST | A_PROTECTED @@ -605,19 +617,24 @@ class First(Builtin): } summary_text = "first element of a list or expression" + # FIXME: the code and the code for Last are similar and can be DRY'd def eval(self, expr, evaluation: Evaluation, expression: Expression): "First[expr__]" if isinstance(expr, Atom): evaluation.message("First", "normal", expression) return - if len(expr.elements) == 0: + expr_len = len(expr.elements) + if expr_len == 0: evaluation.message("First", "nofirst", expr) return - if len(expr.elements) > 2 and expr.head is SymbolSequence: - evaluation.message("First", "argt", len(expr.elements)) + if expr_len > 2 and expr.head is SymbolSequence: + evaluation.message("First", "argt", expr_len) return + if expr_len == 2 and isinstance(expr.elements[0], ListExpression): + return expr.elements[1].evaluate(evaluation) + return expr.elements[0] @@ -819,36 +836,58 @@ class Last(Builtin):
'Last[$expr$]'
returns the last element in $expr$. + +
'Last[$expr$, $def$]' +
returns the last element in $expr$ if it exists or $def$ otherwise.
'Last[$expr$]' is equivalent to '$expr$[[-1]]'. >> Last[{a, b, c}] = c + + The first argument need not be a list: + >> Last[a + b + c] + = c + >> Last[x] : Nonatomic expression expected at position 1 in Last[x]. = Last[x] >> Last[{}] : {} has zero length and no last element. = Last[{}] + + As before, the first argument is empty, but a default argument is given, \ + so evaluate and return the second argument: + >> Last[{}, 1+2] + = 3 """ attributes = A_HOLD_REST | A_PROTECTED messages = { + "argt": "Last called with `1` arguments; 1 or 2 arguments are expected.", "normal": "Nonatomic expression expected at position 1 in `1`.", "nolast": "`1` has zero length and no last element.", } summary_text = "last element of a list or expression" + # FIXME: the code and the code for First are similar and can be DRY'd def eval(self, expr, evaluation: Evaluation, expression: Expression): - "Last[expr_]" + "Last[expr__]" if isinstance(expr, Atom): evaluation.message("Last", "normal", expression) return - if len(expr.elements) == 0: + expr_len = len(expr.elements) + if expr_len == 0: evaluation.message("Last", "nolast", expr) return + if expr_len > 2 and expr.head is SymbolSequence: + evaluation.message("Last", "argt", expr_len) + return + + if expr_len == 2 and isinstance(expr.elements[0], ListExpression): + return expr.elements[1].evaluate(evaluation) return expr.elements[-1] diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index 16d7c9f91..79a4f0bab 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -161,7 +161,13 @@ ("a = {2,3,4}; i = 1; a[[i]] = 0; a", None, "{0, 3, 4}", None), ## Negative step ("{1,2,3,4,5}[[3;;1;;-1]]", None, "{3, 2, 1}", None), - ("{1, 2, 3, 4, 5}[[;; ;; -1]]", None, "{5, 4, 3, 2, 1}", "MMA bug"), + ("ClearAll[a]", None, "Null", None), + ( + "Last[a, b, c]", + ("Last called with 3 arguments; 1 or 2 arguments are expected.",), + "Last[a, b, c]", + None, + ), ("Range[11][[-3 ;; 2 ;; -2]]", None, "{9, 7, 5, 3}", None), ("Range[11][[-3 ;; -7 ;; -3]]", None, "{9, 6}", None), ("Range[11][[7 ;; -7;; -2]]", None, "{7, 5}", None), From d8f398738d633e819ba4a4748e2bfbee6f2213f8 Mon Sep 17 00:00:00 2001 From: rocky Date: Thu, 26 Sep 2024 08:43:09 -0400 Subject: [PATCH 4/5] Handle 2 arg First, Last where 1 arg is an Atom -- this time, for sure https://www.youtube.com/watch?v=pc4IFIXcDcs --- mathics/builtin/list/eol.py | 49 ++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/mathics/builtin/list/eol.py b/mathics/builtin/list/eol.py index 097cf41dc..f3b0d379b 100644 --- a/mathics/builtin/list/eol.py +++ b/mathics/builtin/list/eol.py @@ -595,16 +595,23 @@ class First(Builtin): >> First[a + b + c] = a + However, the first argument must be Nonatomic when there is a single argument: >> First[x] : Nonatomic expression expected at position 1 in First[x]. = First[x] + Or if it is not, but a second default argument is provided, that is \ + evaluated and returned: + + >> First[10, 1+2] + = 3 + >> First[{}] : {} has zero length and no first element. = First[{}] As before, the first argument is empty, but a default argument is given, \ - so evaluate and return the second argument: + evaluate and return the second argument: >> First[{}, 1+2] = 3 """ @@ -632,10 +639,16 @@ def eval(self, expr, evaluation: Evaluation, expression: Expression): evaluation.message("First", "argt", expr_len) return - if expr_len == 2 and isinstance(expr.elements[0], ListExpression): + first_elem = expr.elements[0] + + if expr.head == SymbolSequence or ( + not isinstance(expr, ListExpression) + and len == 2 + and isinstance(first_elem, Atom) + ): return expr.elements[1].evaluate(evaluation) - return expr.elements[0] + return first_elem class FirstCase(Builtin): @@ -850,15 +863,24 @@ class Last(Builtin): >> Last[a + b + c] = c - >> Last[x] - : Nonatomic expression expected at position 1 in Last[x]. - = Last[x] + However, the first argument must be Nonatomic when there is a single argument: + >> Last[10] + : Nonatomic expression expected at position 1 in Last[10]. + = Last[10] + + Or if it is not, but a second default argument is provided, that is \ + evaluated and returned: + + >> Last[10, 1+2] + = 3 + + >> Last[{}] : {} has zero length and no last element. = Last[{}] - As before, the first argument is empty, but a default argument is given, \ - so evaluate and return the second argument: + As before, the first argument is empty, but since default argument is given, \ + evaluate and return the second argument: >> Last[{}, 1+2] = 3 """ @@ -886,10 +908,15 @@ def eval(self, expr, evaluation: Evaluation, expression: Expression): evaluation.message("Last", "argt", expr_len) return - if expr_len == 2 and isinstance(expr.elements[0], ListExpression): - return expr.elements[1].evaluate(evaluation) + last_elem = expr.elements[-1] + if expr.head == SymbolSequence or ( + not isinstance(expr, ListExpression) + and len == 2 + and isinstance(expr.elements[0], Atom) + ): + return last_elem.evaluate(evaluation) - return expr.elements[-1] + return last_elem class Length(Builtin): From c66987687354a3efed76353e784079ce3e19be52 Mon Sep 17 00:00:00 2001 From: rocky Date: Thu, 26 Sep 2024 10:29:35 -0400 Subject: [PATCH 5/5] Remove unneded call to evaluate() --- mathics/builtin/list/eol.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/mathics/builtin/list/eol.py b/mathics/builtin/list/eol.py index f3b0d379b..259f89254 100644 --- a/mathics/builtin/list/eol.py +++ b/mathics/builtin/list/eol.py @@ -646,7 +646,7 @@ def eval(self, expr, evaluation: Evaluation, expression: Expression): and len == 2 and isinstance(first_elem, Atom) ): - return expr.elements[1].evaluate(evaluation) + return expr.elements[1] return first_elem @@ -908,15 +908,7 @@ def eval(self, expr, evaluation: Evaluation, expression: Expression): evaluation.message("Last", "argt", expr_len) return - last_elem = expr.elements[-1] - if expr.head == SymbolSequence or ( - not isinstance(expr, ListExpression) - and len == 2 - and isinstance(expr.elements[0], Atom) - ): - return last_elem.evaluate(evaluation) - - return last_elem + return expr.elements[-1] class Length(Builtin):