From f681eed7a4d17de0cbfa3f98bad9394a44fb5ebe Mon Sep 17 00:00:00 2001 From: Stanislav Date: Thu, 29 Oct 2026 19:04:26 +0400 Subject: [PATCH 01/11] Part assignment: lists of positions, All and negative positions x[[{2, 5}]] = v crashed (SubExpression returned None for a ListExpression position), x[[All, 1]] = v assigned to x[[1]] instead of the first element of every row, x[[-1]] = v wrote the wrong element, and a part that does not exist raised IndexError after some parts had already been changed. Positions are now resolved and checked before anything is replaced: a list of positions, All and a Span select several parts, negative positions count from the end, and a missing part gives Part::partw and leaves the value unchanged. Co-Authored-By: Claude Opus 5.5 --- mathics/core/subexpression.py | 77 ++++++++++++++++++++++++----------- test/builtin/list/test_eol.py | 57 ++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 23 deletions(-) diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index c4b16f99b4..ce3c9ed114 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -59,6 +59,48 @@ def _pspec_span_to_tuple(pspec, expr): return tuple(k for k in range(start, stop, step)) +def _is_atom(expr) -> bool: + """`expr` is an atom; `expr` may also be an `ExpressionPointer` or a `SubExpression`.""" + if isinstance(expr, (ExpressionPointer, SubExpression)): + return expr.is_atom() + return isinstance(expr, Atom) + + +def _part_positions(pspec, expr): + """ + Convert the part specification `pspec` of `expr` into a 1-based + position (0 is the head) when it selects a single part, or into a + tuple of 1-based positions when it selects several (`All`, a list of + integers or a `Span`). Negative positions count from the end. + + Raises `Part::partw` for a position that `expr` does not have. + """ + length = 0 if _is_atom(expr) else len(expr.elements) + + def checked(k: int) -> int: + position = length + 1 + k if k < 0 else k + if not 0 <= position <= length: + target = expr + if type(expr) is ExpressionPointer: + target = expr.parent if expr.position is None else expr.to_expression() + raise MessageException("Part", "partw", Integer(k), target) + return position + + if type(pspec) is Integer: + return checked(pspec.value) + if isinstance(pspec, Symbol) and pspec.get_name() == "System`All": + return tuple(range(1, length + 1)) + if isinstance(pspec, Expression): + if pspec.has_form(SymbolList, None): + positions = [element.int_value for element in pspec.elements] + if any(k is None for k in positions): + raise MessageException("Part", "pspec", pspec) + return tuple(checked(k) for k in positions) + if pspec.has_form(SymbolSpan, None): + return tuple(checked(k + 1) for k in _pspec_span_to_tuple(pspec, expr)) + raise MessageException("Part", "pspec", pspec) + + class ExpressionPointer: """ This class represents a reference to a element in an expression. @@ -121,7 +163,7 @@ def elements(self): if pos is None: return self.parent.elements elif pos == 0: - self.parent.head.elements + return self.parent.head.elements return self.parent.elements[pos - 1].elements @elements.setter @@ -134,10 +176,10 @@ def get_head_name(self): def is_atom(self): pos = self.position if pos is None: - return self.parent.is_atom() + return _is_atom(self.parent) elif pos == 0: - return self.parent.head.is_atom() - return self.parent.elements[pos - 1].is_atom() + return _is_atom(self.parent.head) + return _is_atom(self.parent.elements[pos - 1]) def to_expression(self): parent = self.parent @@ -226,34 +268,23 @@ def __new__(cls, expr, pos=None): else: rem_pos = None - # Trivial conversion: if pos is an `Integer`, convert - # to a Python native int - if type(pos) is Integer: - pos = pos.int_value - # pos == `System`All` - elif isinstance(pos, Symbol) and pos.get_name() == "System`All": - pos = None - elif type(pos) is Expression: - if pos.has_form(SymbolList, None): - tuple_pos = [i.int_value for i in pos.elements] - if any(i is None for i in tuple_pos): - raise MessageException("Part", "pspec", pos) - pos = tuple_pos - elif pos.has_form(SymbolSpan, None): - pos = _pspec_span_to_tuple(pos, expr) - else: - raise MessageException("Part", "pspec", pos) + # Convert the part specification into a 1-based position (0 is + # the head), or a tuple of them. Every position is checked here, + # before anything is replaced, so that a part that does not exist + # leaves the expression unchanged. + if pos is not None: + pos = _part_positions(pos, expr) if pos is None or type(pos) is int: if rem_pos is None: return ExpressionPointer(expr, pos) else: return SubExpression(ExpressionPointer(expr, pos), rem_pos) - elif type(pos) is tuple: + else: self = super(SubExpression, cls).__new__(cls) self._headp = ExpressionPointer(expr.head, 0) self._elementsp = [ - SubExpression(ExpressionPointer(expr, k + 1), rem_pos) for k in pos + SubExpression(ExpressionPointer(expr, k), rem_pos) for k in pos ] return self diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index 0740fb66a9..f1c69331ba 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -470,3 +470,60 @@ def test_extract(str_expr, expected_messages, str_expected): def test_extract_arg_errors(): check_arg_counts("Extract", "2 or 3 arguments are") + + +@pytest.mark.parametrize( + ("str_expr", "expected_messages", "str_expected"), + [ + ("pa = Range[8]; pa[[{2, 5, 6}]] = {-1, -2, -3}; pa", None, "{1, -1, 3, 4, -2, -3, 7, 8}"), + ("pa = Range[8]; pa[[{2, 3}]] = 0; pa", None, "{1, 0, 0, 4, 5, 6, 7, 8}"), + ("pa = Range[8]; pa[[{-1, 1}]] = {p, q}; pa", None, "{q, 2, 3, 4, 5, 6, 7, p}"), + # A right side of another length is assigned to each part whole. + ( + "pa = Range[4]; pa[[{1, 2}]] = {u, v, w}; pa", + None, + "{{u, v, w}, {u, v, w}, 3, 4}", + ), + ("pa = Range[3]; pa[[All]] = 0; pa", None, "{0, 0, 0}"), + ("pa = Range[5]; pa[[2 ;; 4]] = {a, b, c}; pa", None, "{1, a, b, c, 5}"), + ("pa = Range[3]; pa[[-1]] = z; pa", None, "{1, 2, z}"), + ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = {a, b}; pm", None, "{{a, 2}, {b, 4}}"), + ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = z; pm", None, "{{z, 2}, {z, 4}}"), + ("pm = {{1, 2}, {3, 4}}; pm[[{1, 2}, 2]] = {x, y}; pm", None, "{{1, x}, {3, y}}"), + ("pm = {{1, 2}, {3, 4}}; pm[[2, All]] = {x, y}; pm", None, "{{1, 2}, {x, y}}"), + ( + "pm = {{1, 2}, {3, 4}}; pm[[{2, 1}, {1}]] = {{a}, {b}}; pm", + None, + "{{b, 2}, {a, 4}}", + ), + ("pg = f[a, b, c]; pg[[{1, 3}]] = {x, y}; pg", None, "f[x, b, y]"), + ("pg = f[a]; pg[[0]] = h; pg", None, "h[a]"), + # A part that does not exist leaves the whole value unchanged. + ( + "pa = Range[3]; pa[[{1, 5}]] = 0; pa", + ("Part 5 of {1, 2, 3} does not exist.",), + "{1, 2, 3}", + ), + ( + "pa = Range[3]; pa[[2 ;; 5]] = 0; pa", + ("Part 4 of {1, 2, 3} does not exist.",), + "{1, 2, 3}", + ), + ( + "pm = {{1, 2}, {3, 4}}; pm[[1, 5]] = 0; pm", + ("Part 5 of {1, 2} does not exist.",), + "{{1, 2}, {3, 4}}", + ), + ], +) +def test_part_assignment(str_expr, expected_messages, str_expected): + """Assigning to parts given by a list of positions, All, a Span or a + negative position, at any depth.""" + check_evaluation( + str_expr, + str_expected, + to_string_expr=True, + to_string_expected=True, + hold_expected=True, + expected_messages=expected_messages, + ) From 469e85dcde099bb67199e02ea267e7fb976d6dd7 Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 03:08:04 +0400 Subject: [PATCH 02/11] Part assignment tests: symbols no other test sets Co-Authored-By: Claude Opus 5.5 --- test/builtin/list/test_eol.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index f1c69331ba..1d07f7e0e8 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -489,14 +489,14 @@ def test_extract_arg_errors(): ("pa = Range[3]; pa[[-1]] = z; pa", None, "{1, 2, z}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = {a, b}; pm", None, "{{a, 2}, {b, 4}}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = z; pm", None, "{{z, 2}, {z, 4}}"), - ("pm = {{1, 2}, {3, 4}}; pm[[{1, 2}, 2]] = {x, y}; pm", None, "{{1, x}, {3, y}}"), - ("pm = {{1, 2}, {3, 4}}; pm[[2, All]] = {x, y}; pm", None, "{{1, 2}, {x, y}}"), + ("pm = {{1, 2}, {3, 4}}; pm[[{1, 2}, 2]] = {pax, pay}; pm", None, "{{1, pax}, {3, pay}}"), + ("pm = {{1, 2}, {3, 4}}; pm[[2, All]] = {pax, pay}; pm", None, "{{1, 2}, {pax, pay}}"), ( "pm = {{1, 2}, {3, 4}}; pm[[{2, 1}, {1}]] = {{a}, {b}}; pm", None, "{{b, 2}, {a, 4}}", ), - ("pg = f[a, b, c]; pg[[{1, 3}]] = {x, y}; pg", None, "f[x, b, y]"), + ("pg = f[a, b, c]; pg[[{1, 3}]] = {pax, pay}; pg", None, "f[pax, b, pay]"), ("pg = f[a]; pg[[0]] = h; pg", None, "h[a]"), # A part that does not exist leaves the whole value unchanged. ( From 89ebf7e9ced092a7d29548ec75d19de99f474c4b Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 03:20:12 +0400 Subject: [PATCH 03/11] Part assignment to position 0 of a List no longer crashes l[[0]] = f with l a List, m[[2, 0]] = g on a nested List, and k[[0]] = List crashed with a TypeError: a List cannot take another head in place. The part is now replaced by a new expression with the new head. The value assigned to is held while it changes, so that the value itself can be replaced as well. Co-Authored-By: Claude Opus 5.5 --- mathics/core/subexpression.py | 20 +++++++++++++++----- mathics/eval/assignments/assignment.py | 9 ++++++++- test/builtin/list/test_eol.py | 4 ++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index ce3c9ed114..4674974b21 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -9,6 +9,7 @@ from mathics.core.atoms import Integer from mathics.core.exceptions import MessageException from mathics.core.expression import Expression +from mathics.core.list import ListExpression from mathics.core.symbols import Atom, Symbol, SymbolList from mathics.core.systemsymbols import SymbolSpan @@ -217,12 +218,12 @@ def replace(self, new): # Variables to track the parent and the position of that parent. current = parent current_parent = None - # current_index = None + current_index = None try: while pos: current_parent = current - # current_index = i + current_index = i if i == 0: current = current._head else: @@ -233,10 +234,19 @@ def replace(self, new): # Now, current is the subexpression to be modified, and current_parent # is the expression that holds it (if exists). - if i == 0: - current.set_head(new) - else: + if i != 0: current.set_element(i - 1, new) + elif isinstance(current, ListExpression) or new is SymbolList: + # A List cannot take another head, nor an Expression the head + # List: a new expression takes the place of the old one. + replacement = Expression(new, *current.elements) + if new is SymbolList: + replacement = ListExpression(*current.elements) + if current_parent is None or current_index == 0: + raise MessageException("Part", "partw", Integer(0), current) + current_parent.set_element(current_index - 1, replacement) + else: + current.set_head(new) # If the modified subexpression is not the root, # invalidate the parent cache. diff --git a/mathics/eval/assignments/assignment.py b/mathics/eval/assignments/assignment.py index 108b1347be..07a226c190 100644 --- a/mathics/eval/assignments/assignment.py +++ b/mathics/eval/assignments/assignment.py @@ -44,6 +44,7 @@ SymbolCondition, SymbolDefault, SymbolDirectedInfinity, + SymbolHold, SymbolHoldPattern, SymbolMachinePrecision, SymbolMakeBoxes, @@ -1128,7 +1129,13 @@ def eval_assign_part( evaluation.message(op_name, "noval", lhs_symbol) return False indices = lhs.elements[1:] - return eval_Part([rule], indices, evaluation, rhs) + # The value is changed in place, inside a holder so that the value + # itself can be replaced too: l[[0]] = f turns the List l into f[...]. + holder = Expression(SymbolHold, rule) + result = eval_Part([holder], [Integer1, *indices], evaluation, rhs) + if holder.elements[0] is not rule: + defs.set_ownvalue(lhs_name, holder.elements[0]) + return result def eval_assign_random_state( diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index 1d07f7e0e8..f8f18fe93a 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -509,6 +509,10 @@ def test_extract_arg_errors(): ("Part 4 of {1, 2, 3} does not exist.",), "{1, 2, 3}", ), + ("pl = {1, 2, 3}; pl[[0]] = pf; pl", None, "pf[1, 2, 3]"), + ("pm = {{1, 2}, {3, 4}}; pm[[2, 0]] = phg; pm", None, "{{1, 2}, phg[3, 4]}"), + ("pm = {{1, 2}, {3, 4}}; pm[[All, 0]] = ph; pm", None, "{ph[1, 2], ph[3, 4]}"), + ("pk = pf[1, 2]; pk[[0]] = List; pk", None, "{1, 2}"), ( "pm = {{1, 2}, {3, 4}}; pm[[1, 5]] = 0; pm", ("Part 5 of {1, 2} does not exist.",), From f9bdf6ba3176db6e20221efd79024bfbe700bd3b Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 10:19:40 +0400 Subject: [PATCH 04/11] Part assignment changes only the variable, never a value it shares b = a; b[[2, 1]] = 9 changed a too, and k[] := (w = {1, {2}}; w[[2, 1]] = 5; w) changed the definition of k: the parts on the way to the assigned position were changed in place, though they may be shared with other values and definitions. They are now copied first. A ListExpression changed with set_element works out again whether it is literal. Span positions counted from the end, and spans with a negative step, now select the parts WMA selects. Co-Authored-By: Claude Opus 5.5 --- mathics/core/list.py | 12 +++++++++ mathics/core/subexpression.py | 48 ++++++++++++++++++++--------------- test/builtin/list/test_eol.py | 17 +++++++++++++ 3 files changed, 56 insertions(+), 21 deletions(-) diff --git a/mathics/core/list.py b/mathics/core/list.py index d5ce93b1b0..6e62ad3602 100644 --- a/mathics/core/list.py +++ b/mathics/core/list.py @@ -121,6 +121,18 @@ def evaluate_elements(self, evaluation: Evaluation) -> Expression: new_list._build_elements_properties() return new_list + def set_element(self, index: int, value): + """ + Update element[i] with value. Whether the list is literal, and its + Python value, are worked out again from the new elements. + """ + super().set_element(index, value) + self.elements_properties = None + self.value = None + self._is_literal = all(element.is_literal for element in self._elements) + if self._is_literal: + self.value = tuple(element.value for element in self._elements) + @property def is_literal(self) -> bool: """ diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index 4674974b21..6e554cc151 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -20,44 +20,41 @@ def _pspec_span_to_tuple(pspec, expr): `Span` Expression and returns a tuple with the positions of the elements. """ - start = 1 - stop = None - step = 1 elements = pspec.elements if len(elements) > 3: raise MessageException("Part", "span", elements) - if len(elements) > 0: - start = elements[0].int_value + length = len(expr.elements) + start = elements[0].int_value if len(elements) > 0 else 1 + stop = length if len(elements) > 1: stop = elements[1].int_value if stop is None: - if elements[1].get_name() == "System`All": - stop = None - else: + if elements[1].get_name() != "System`All": raise MessageException("Part", "span", pspec) - else: - stop = stop - 1 if stop > 0 else len(expr.elements) + stop - - if len(pspec.elements) > 2: - step = elements[2].int_value + stop = length + step = elements[2].int_value if len(elements) > 2 else 1 - if start is None or step is None: + if start is None or step is None or step == 0: raise MessageException("Part", "span", pspec) if start == 0 or stop == 0: # index 0 is undefined raise MessageException("Part", "span", Integer(0)) + # Negative positions count from the end; the tuple is 0-based. if start < 0: - start = len(expr.elements) - start - else: - start = start - 1 + start += length + 1 + if stop < 0: + stop += length + 1 + return tuple(k - 1 for k in range(start, stop + (1 if step > 0 else -1), step)) - if stop is None: - stop = 0 if step < 0 else len(expr.elements) - 1 - stop = stop + 1 if step > 0 else stop - 1 - return tuple(k for k in range(start, stop, step)) +def _copy(expr: Expression) -> Expression: + """A new expression with the head and elements of `expr`, to be changed + in place of `expr`.""" + if isinstance(expr, ListExpression): + return ListExpression(*expr.elements) + return Expression(expr.head, *expr.elements) def _is_atom(expr) -> bool: @@ -220,14 +217,23 @@ def replace(self, new): current_parent = None current_index = None + # Each expression on the way down is replaced by a copy before it + # is changed: it may be shared with another value or definition, + # as in b = a; b[[2, 1]] = x, which must leave a as it was. try: while pos: current_parent = current current_index = i if i == 0: current = current._head + if isinstance(current, Expression): + current = _copy(current) + current_parent.set_head(current) else: current = current.elements[i - 1] + if isinstance(current, Expression): + current = _copy(current) + current_parent.set_element(i - 1, current) i = pos.pop() except IndexError: raise MessageException("Part", "span", pos) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index f8f18fe93a..bebf181f28 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -518,6 +518,23 @@ def test_extract_arg_errors(): ("Part 5 of {1, 2} does not exist.",), "{{1, 2}, {3, 4}}", ), + ("pm = {{1, 2}, {3, 4}, {5, 6}}; pm[[-2 ;; -1, 2]] = 0; pm", None, "{{1, 2}, {3, 0}, {5, 0}}"), + ("pa = Range[5]; pa[[-1 ;; 1 ;; -2]] = 0; pa", None, "{0, 2, 0, 4, 0}"), + ("pa = Range[5]; pa[[4 ;; 2 ;; -1]] = {u, v, w}; pa", None, "{1, w, v, u, 5}"), + # The value assigned to may be shared: only the variable changes. + ("pa = {1, {2}}; pb = pa; pb[[2, 1]] = 9; {pa, pb}", None, "{{1, {2}}, {1, {9}}}"), + ( + "pa = {{1, 2}, {3, 4}}; pb = pa; pb[[All, 1]] = 0; {pa, pb}", + None, + "{{{1, 2}, {3, 4}}, {{0, 2}, {0, 4}}}", + ), + ( + "pkd[] := (pw = {1, {2}}; pw[[2, 1]] = 5; pw); " + "{pkd[], pkd[], DownValues[pkd] === " + "{HoldPattern[pkd[]] :> (pw = {1, {2}}; pw[[2, 1]] = 5; pw)}}", + None, + "{{1, {5}}, {1, {5}}, True}", + ), ], ) def test_part_assignment(str_expr, expected_messages, str_expected): From 2e39f5f57b043a7d37e5f251c3aaeaa97dc25c26 Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 10:43:59 +0400 Subject: [PATCH 05/11] Part assignment: a position list with a non-integer gives pkspec1 pa[[{2, f[x]}]] = 5 raised an AttributeError; it now gives the message WMA gives and leaves pa unchanged. Co-Authored-By: Claude Opus 5.5 --- mathics/core/subexpression.py | 7 +++---- test/builtin/list/test_eol.py | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index 4674974b21..fdd21f9ec2 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -93,10 +93,9 @@ def checked(k: int) -> int: return tuple(range(1, length + 1)) if isinstance(pspec, Expression): if pspec.has_form(SymbolList, None): - positions = [element.int_value for element in pspec.elements] - if any(k is None for k in positions): - raise MessageException("Part", "pspec", pspec) - return tuple(checked(k) for k in positions) + if not all(type(element) is Integer for element in pspec.elements): + raise MessageException("Part", "pkspec1", pspec) + return tuple(checked(element.value) for element in pspec.elements) if pspec.has_form(SymbolSpan, None): return tuple(checked(k + 1) for k in _pspec_span_to_tuple(pspec, expr)) raise MessageException("Part", "pspec", pspec) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index f8f18fe93a..d28b9c3494 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -513,6 +513,11 @@ def test_extract_arg_errors(): ("pm = {{1, 2}, {3, 4}}; pm[[2, 0]] = phg; pm", None, "{{1, 2}, phg[3, 4]}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 0]] = ph; pm", None, "{ph[1, 2], ph[3, 4]}"), ("pk = pf[1, 2]; pk[[0]] = List; pk", None, "{1, 2}"), + ( + "pa = {0, 0, 0}; pa[[{2, pf[pq]}]] = 5; pa", + ("The expression {2, pf[pq]} cannot be used as a part specification.",), + "{0, 0, 0}", + ), ( "pm = {{1, 2}, {3, 4}}; pm[[1, 5]] = 0; pm", ("Part 5 of {1, 2} does not exist.",), From cf975239c1aa199da7420ed70138af8e5bc6f23e Mon Sep 17 00:00:00 2001 From: rocky Date: Fri, 30 Oct 2026 07:18:42 -0400 Subject: [PATCH 06/11] black --- test/builtin/list/test_eol.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index d28b9c3494..6da7500bac 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -475,7 +475,11 @@ def test_extract_arg_errors(): @pytest.mark.parametrize( ("str_expr", "expected_messages", "str_expected"), [ - ("pa = Range[8]; pa[[{2, 5, 6}]] = {-1, -2, -3}; pa", None, "{1, -1, 3, 4, -2, -3, 7, 8}"), + ( + "pa = Range[8]; pa[[{2, 5, 6}]] = {-1, -2, -3}; pa", + None, + "{1, -1, 3, 4, -2, -3, 7, 8}", + ), ("pa = Range[8]; pa[[{2, 3}]] = 0; pa", None, "{1, 0, 0, 4, 5, 6, 7, 8}"), ("pa = Range[8]; pa[[{-1, 1}]] = {p, q}; pa", None, "{q, 2, 3, 4, 5, 6, 7, p}"), # A right side of another length is assigned to each part whole. @@ -489,8 +493,16 @@ def test_extract_arg_errors(): ("pa = Range[3]; pa[[-1]] = z; pa", None, "{1, 2, z}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = {a, b}; pm", None, "{{a, 2}, {b, 4}}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 1]] = z; pm", None, "{{z, 2}, {z, 4}}"), - ("pm = {{1, 2}, {3, 4}}; pm[[{1, 2}, 2]] = {pax, pay}; pm", None, "{{1, pax}, {3, pay}}"), - ("pm = {{1, 2}, {3, 4}}; pm[[2, All]] = {pax, pay}; pm", None, "{{1, 2}, {pax, pay}}"), + ( + "pm = {{1, 2}, {3, 4}}; pm[[{1, 2}, 2]] = {pax, pay}; pm", + None, + "{{1, pax}, {3, pay}}", + ), + ( + "pm = {{1, 2}, {3, 4}}; pm[[2, All]] = {pax, pay}; pm", + None, + "{{1, 2}, {pax, pay}}", + ), ( "pm = {{1, 2}, {3, 4}}; pm[[{2, 1}, {1}]] = {{a}, {b}}; pm", None, From 50311e1bca94201883fba795448c2f2bbf78e2ee Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 15:34:33 +0400 Subject: [PATCH 07/11] Part assignment keeps what is known of a list instead of going over it again Each p[[i]] = v on a long list went over all its elements several times: set_element worked out again whether the list is literal, and afterwards its elements properties and its symbol cache were rebuilt, and the list was evaluated element by element, because they had been cleared. ListExpression.set_element now updates the literal value, the elements properties and the cache from the old ones and the new element; copies of a list keep them too. A Fisher-Yates shuffle of Range[1000] by Part assignments (Combinatorica's RandomPermutation) took 11.7 s with the copy on write of this branch and 9.3 s on master; it now takes 2.8 s. Co-Authored-By: Claude Opus 5.5 --- mathics/core/list.py | 58 +++++++++++++++++++++++++++++------ mathics/core/subexpression.py | 7 ++++- test/builtin/list/test_eol.py | 4 +++ 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/mathics/core/list.py b/mathics/core/list.py index 6e62ad3602..1957001d60 100644 --- a/mathics/core/list.py +++ b/mathics/core/list.py @@ -8,7 +8,7 @@ from mathics.core.element import ElementsProperties from mathics.core.evaluation import Evaluation -from mathics.core.expression import Expression +from mathics.core.expression import Expression, ExpressionCache from mathics.core.symbols import EvalMixin, Symbol, SymbolList from mathics.core.systemsymbols import SymbolUnevaluated @@ -123,15 +123,53 @@ def evaluate_elements(self, evaluation: Evaluation) -> Expression: def set_element(self, index: int, value): """ - Update element[i] with value. Whether the list is literal, and its - Python value, are worked out again from the new elements. + Update element[i] with value. Whether the list is literal, its + Python value and its elements properties are updated from the old + ones and the new element, without going over all the elements: a + loop such as p[[i]] = p[[j]] on a long list does this at each step. """ + was_literal = self._is_literal + properties = self.elements_properties + cache = self._cache super().set_element(index, value) - self.elements_properties = None - self.value = None - self._is_literal = all(element.is_literal for element in self._elements) - if self._is_literal: - self.value = tuple(element.value for element in self._elements) + if cache is not None and cache.symbols is not None and cache.sequences is not None: + # The symbols of the old element are kept: a set that has more + # symbols than the list only costs a check. There is no + # timestamp: the list has changed. + symbols = set(cache.symbols) + sequences = [i for i in cache.sequences if i != index] + if isinstance(value, Expression): + value_symbols = value._rebuild_cache().symbols + symbols.update(value_symbols) + if "System`Sequence" in value_symbols: + sequences = sorted(sequences + [index]) + elif isinstance(value, Symbol): + symbols.add(value.get_name()) + self._cache = ExpressionCache(None, symbols, sequences) + if not value.is_literal: + self._is_literal = False + self.value = None + elif was_literal: + values = list(self.value) + values[index] = value.value + self.value = tuple(values) + else: + # The element replaced may have been the only one not literal. + self._is_literal = all(element.is_literal for element in self._elements) + self.value = ( + tuple(element.value for element in self._elements) + if self._is_literal + else None + ) + if properties is not None: + # False is always safe for a property: it only means "not known". + self.elements_properties = ElementsProperties( + elements_fully_evaluated=properties.elements_fully_evaluated + and value.is_literal, + is_flat=properties.is_flat and not isinstance(value, Expression), + is_ordered=False, + is_uniform=False, + ) @property def is_literal(self) -> bool: @@ -184,7 +222,9 @@ def shallow_copy(self) -> "ListExpression": Here this does not need that complication. """ return ListExpression( - *self._elements, elements_properties=self.elements_properties + *self._elements, + elements_properties=self.elements_properties, + literal_values=self.value if self._is_literal else None, ) def copy(self, reevaluate=False) -> "Expression": diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index c34a805574..bbb2ac2a99 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -53,7 +53,12 @@ def _copy(expr: Expression) -> Expression: """A new expression with the head and elements of `expr`, to be changed in place of `expr`.""" if isinstance(expr, ListExpression): - return ListExpression(*expr.elements) + # What is known of the elements stays true of the copy. + return ListExpression( + *expr.elements, + elements_properties=expr.elements_properties, + literal_values=expr.value if expr.is_literal else None, + ) return Expression(expr.head, *expr.elements) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index d521ae797c..98d5b5e290 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -526,6 +526,10 @@ def test_extract_arg_errors(): ("pm = {{1, 2}, {3, 4}, {5, 6}}; pm[[-2 ;; -1, 2]] = 0; pm", None, "{{1, 2}, {3, 0}, {5, 0}}"), ("pa = Range[5]; pa[[-1 ;; 1 ;; -2]] = 0; pa", None, "{0, 2, 0, 4, 0}"), ("pa = Range[5]; pa[[4 ;; 2 ;; -1]] = {u, v, w}; pa", None, "{1, w, v, u, 5}"), + # A part assigned a symbol is evaluated again once the symbol has a value. + ("ClearAll[psx]; pa = {1, 2, 3}; pa[[2]] = psx; psx = 5; pa", None, "{1, 5, 3}"), + ("pa = {1, 2}; pa[[1]] = Sequence[pu, pv]; pa", None, "{pu, pv, 2}"), + ("pa = {pu, 2}; pa[[1]] = 7; {pa, Total[pa]}", None, "{{7, 2}, 9}"), # The value assigned to may be shared: only the variable changes. ("pa = {1, {2}}; pb = pa; pb[[2, 1]] = 9; {pa, pb}", None, "{{1, {2}}, {1, {9}}}"), ( From da632bce9d04455e045f210f1f6e10a0c80b8832 Mon Sep 17 00:00:00 2001 From: Stanislav Date: Fri, 30 Oct 2026 15:45:16 +0400 Subject: [PATCH 08/11] Part assignment: a negative position past the start gives partw part[[-4]] = 0 on a list of three elements turned -4 into position 0 and replaced the head, giving 0[1, 2, 3]. A negative position now has to be between -Length and -1, as in WMA; position 0 is reached only as 0. Co-Authored-By: Claude Opus 5.5 --- mathics/core/subexpression.py | 4 +++- test/builtin/list/test_eol.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index fdd21f9ec2..06d6140991 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -80,7 +80,9 @@ def _part_positions(pspec, expr): def checked(k: int) -> int: position = length + 1 + k if k < 0 else k - if not 0 <= position <= length: + # Position 0 is the head; a negative position never reaches it: + # -4 in a list of 3 does not exist. + if not (1 if k < 0 else 0) <= position <= length: target = expr if type(expr) is ExpressionPointer: target = expr.parent if expr.position is None else expr.to_expression() diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index 6da7500bac..47e9793af5 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -521,6 +521,18 @@ def test_extract_arg_errors(): ("Part 4 of {1, 2, 3} does not exist.",), "{1, 2, 3}", ), + # A negative position never reaches the head at position 0. + ( + "pa = {1, 2, 3}; pa[[-4]] = 0; pa", + ("Part -4 of {1, 2, 3} does not exist.",), + "{1, 2, 3}", + ), + ( + "pa = {1, 2, 3}; pa[[{-4, 2}]] = {pax, pay}; pa", + ("Part -4 of {1, 2, 3} does not exist.",), + "{1, 2, 3}", + ), + ("pa = {1, 2, 3}; pa[[-3]] = pax; pa", None, "{pax, 2, 3}"), ("pl = {1, 2, 3}; pl[[0]] = pf; pl", None, "pf[1, 2, 3]"), ("pm = {{1, 2}, {3, 4}}; pm[[2, 0]] = phg; pm", None, "{{1, 2}, phg[3, 4]}"), ("pm = {{1, 2}, {3, 4}}; pm[[All, 0]] = ph; pm", None, "{ph[1, 2], ph[3, 4]}"), From e10f76fe27859de8c5cb1027f322d8f5c3541a8e Mon Sep 17 00:00:00 2001 From: rocky Date: Fri, 30 Oct 2026 07:47:09 -0400 Subject: [PATCH 09/11] black --- test/builtin/list/test_eol.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/builtin/list/test_eol.py b/test/builtin/list/test_eol.py index 52ae2e37d0..85d56ec6db 100644 --- a/test/builtin/list/test_eol.py +++ b/test/builtin/list/test_eol.py @@ -535,15 +535,27 @@ def test_extract_arg_errors(): ("Part 5 of {1, 2} does not exist.",), "{{1, 2}, {3, 4}}", ), - ("pm = {{1, 2}, {3, 4}, {5, 6}}; pm[[-2 ;; -1, 2]] = 0; pm", None, "{{1, 2}, {3, 0}, {5, 0}}"), + ( + "pm = {{1, 2}, {3, 4}, {5, 6}}; pm[[-2 ;; -1, 2]] = 0; pm", + None, + "{{1, 2}, {3, 0}, {5, 0}}", + ), ("pa = Range[5]; pa[[-1 ;; 1 ;; -2]] = 0; pa", None, "{0, 2, 0, 4, 0}"), ("pa = Range[5]; pa[[4 ;; 2 ;; -1]] = {u, v, w}; pa", None, "{1, w, v, u, 5}"), # A part assigned a symbol is evaluated again once the symbol has a value. - ("ClearAll[psx]; pa = {1, 2, 3}; pa[[2]] = psx; psx = 5; pa", None, "{1, 5, 3}"), + ( + "ClearAll[psx]; pa = {1, 2, 3}; pa[[2]] = psx; psx = 5; pa", + None, + "{1, 5, 3}", + ), ("pa = {1, 2}; pa[[1]] = Sequence[pu, pv]; pa", None, "{pu, pv, 2}"), ("pa = {pu, 2}; pa[[1]] = 7; {pa, Total[pa]}", None, "{{7, 2}, 9}"), # The value assigned to may be shared: only the variable changes. - ("pa = {1, {2}}; pb = pa; pb[[2, 1]] = 9; {pa, pb}", None, "{{1, {2}}, {1, {9}}}"), + ( + "pa = {1, {2}}; pb = pa; pb[[2, 1]] = 9; {pa, pb}", + None, + "{{1, {2}}, {1, {9}}}", + ), ( "pa = {{1, 2}, {3, 4}}; pb = pa; pb[[All, 1]] = 0; {pa, pb}", None, From 5e171a67ad90ca8249b8d35cf7966f473bcbea67 Mon Sep 17 00:00:00 2001 From: rocky Date: Sat, 31 Oct 2026 09:49:08 -0400 Subject: [PATCH 10/11] black and mypy --- mathics/core/list.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/mathics/core/list.py b/mathics/core/list.py index 1957001d60..232e6bb34f 100644 --- a/mathics/core/list.py +++ b/mathics/core/list.py @@ -1,10 +1,9 @@ -# -*- coding: utf-8 -*- """ Module containing ListExpression """ import reprlib -from typing import Any, Optional, Tuple +from typing import Any from mathics.core.element import ElementsProperties from mathics.core.evaluation import Evaluation @@ -30,13 +29,13 @@ class ListExpression(Expression): """ _is_literal: bool - _sympy: Optional[Any] + _sympy: Any | None def __init__( self, *elements, - elements_properties: Optional[ElementsProperties] = None, - literal_values: Optional[tuple] = None, + elements_properties: ElementsProperties | None = None, + literal_values: tuple | None = None, ): self.options = None self.pattern_sequence = False @@ -106,13 +105,14 @@ def evaluate_elements(self, evaluation: Evaluation) -> Expression: # Make tuple self._elements mutable by turning it into a list. elements = list(self._elements) for index, element in enumerate(self._elements): - if not element.has_form(SymbolUnevaluated, 1): - if isinstance(element, EvalMixin): - new_value = element.evaluate(evaluation) - # We need id() because != by itself is too permissive - if new_value is not None and id(element) != id(new_value): - elements_changed = True - elements[index] = new_value + if not element.has_form(SymbolUnevaluated, 1) and isinstance( + element, EvalMixin + ): + new_value = element.evaluate(evaluation) + # We need id() because != by itself is too permissive + if new_value is not None and id(element) != id(new_value): + elements_changed = True + elements[index] = new_value if not elements_changed: return self @@ -132,7 +132,11 @@ def set_element(self, index: int, value): properties = self.elements_properties cache = self._cache super().set_element(index, value) - if cache is not None and cache.symbols is not None and cache.sequences is not None: + if ( + cache is not None + and cache.symbols is not None + and cache.sequences is not None + ): # The symbols of the old element are kept: a set that has more # symbols than the list only costs a check. There is no # timestamp: the list has changed. @@ -150,15 +154,15 @@ def set_element(self, index: int, value): self._is_literal = False self.value = None elif was_literal: - values = list(self.value) + values = list(self.value or ()) values[index] = value.value self.value = tuple(values) else: # The element replaced may have been the only one not literal. - self._is_literal = all(element.is_literal for element in self._elements) self.value = ( - tuple(element.value for element in self._elements) - if self._is_literal + tuple(vals) + if len(vals := [e.value for e in self._elements if hasattr(e, "value")]) + == len(self._elements) else None ) if properties is not None: @@ -181,7 +185,7 @@ def is_literal(self) -> bool: """ return self._is_literal - def rewrite_apply_eval_step(self, evaluation) -> Tuple[Expression, bool]: + def rewrite_apply_eval_step(self, evaluation) -> tuple[Expression, bool]: """ Perform a single rewrite/apply/eval step of the bigger Expression.evaluate() process. From 82df6d098aca0bc1dc8774a7e7953d31df0ebf95 Mon Sep 17 00:00:00 2001 From: rocky Date: Sat, 31 Oct 2026 09:55:53 -0400 Subject: [PATCH 11/11] Remove duplicate functions Probably introduced via merge. --- mathics/core/subexpression.py | 44 ----------------------------------- 1 file changed, 44 deletions(-) diff --git a/mathics/core/subexpression.py b/mathics/core/subexpression.py index d228f869be..631cac4a9c 100644 --- a/mathics/core/subexpression.py +++ b/mathics/core/subexpression.py @@ -1,5 +1,4 @@ # cython: language_level=3 -# -*- coding: utf-8 -*- """ This module provides some infrastructure to deal with SubExpressions. @@ -105,49 +104,6 @@ def checked(k: int) -> int: raise MessageException("Part", "pspec", pspec) -def _is_atom(expr) -> bool: - """`expr` is an atom; `expr` may also be an `ExpressionPointer` or a `SubExpression`.""" - if isinstance(expr, (ExpressionPointer, SubExpression)): - return expr.is_atom() - return isinstance(expr, Atom) - - -def _part_positions(pspec, expr): - """ - Convert the part specification `pspec` of `expr` into a 1-based - position (0 is the head) when it selects a single part, or into a - tuple of 1-based positions when it selects several (`All`, a list of - integers or a `Span`). Negative positions count from the end. - - Raises `Part::partw` for a position that `expr` does not have. - """ - length = 0 if _is_atom(expr) else len(expr.elements) - - def checked(k: int) -> int: - position = length + 1 + k if k < 0 else k - # Position 0 is the head; a negative position never reaches it: - # -4 in a list of 3 does not exist. - if not (1 if k < 0 else 0) <= position <= length: - target = expr - if type(expr) is ExpressionPointer: - target = expr.parent if expr.position is None else expr.to_expression() - raise MessageException("Part", "partw", Integer(k), target) - return position - - if type(pspec) is Integer: - return checked(pspec.value) - if isinstance(pspec, Symbol) and pspec.get_name() == "System`All": - return tuple(range(1, length + 1)) - if isinstance(pspec, Expression): - if pspec.has_form(SymbolList, None): - if not all(type(element) is Integer for element in pspec.elements): - raise MessageException("Part", "pkspec1", pspec) - return tuple(checked(element.value) for element in pspec.elements) - if pspec.has_form(SymbolSpan, None): - return tuple(checked(k + 1) for k in _pspec_span_to_tuple(pspec, expr)) - raise MessageException("Part", "pspec", pspec) - - class ExpressionPointer: """ This class represents a reference to a element in an expression.