improve `OneIdentity` builtin #602

mmatera · 2022-11-10T00:52:21Z

OneIdentity is an attribute that allows to implement of the property of Idempotency in the pattern matching mechanism.

https://reference.wolfram.com/language/ref/OneIdentity.html
https://mathematica.stackexchange.com/questions/124372/about-oneidentity

Documentation of this attribute in WR is not very complete, so the previous implementation was the result of a poor understanding of the expected behavior. This also forced us to put a lot of hacks in order to make basic things like assignments to work.

This PR simplifies a lot this behavior, removing unnecessary parameters in the match methods, and provides a behavior closer to the expected WL. Also, several new pytests were included.

In the process, I started to add comments on the mathics.core.pattern module to make it easier to understand and follow (I still do not do it fully).

mathics/builtin/assignments/internals.py

mmatera · 2022-11-10T00:54:40Z

mathics/builtin/attributes.py

@@ -476,9 +476,14 @@ class OneIdentity(Predefined):

    'OneIdentity' affects pattern matching:
    >> SetAttributes[f, OneIdentity]
-    >> a /. f[args___] -> {args}
+    >> a /. f[x_:0, u_] -> {u}


Reading the documentation, I realize that this test was wrong...

Good catch! This bug goes back to the very first Mathics version.

mathics/builtin/files_io/files.py

mathics/builtin/options.py

mmatera · 2022-11-10T00:55:51Z

mathics/core/evaluation.py

@@ -262,9 +262,6 @@ def __init__(
        # status of last evaluate
        self.exc_result = self.SymbolNull
        self.last_eval = None
-        # Necesary to handle OneIdentity on
-        # lhs in assignment
-        self.ignore_oneidentity = False


mathics/core/pattern.py

mmatera · 2022-11-10T00:57:35Z

test/test_context.py

-def test_context2():
-    nomessage = tuple([])
-    for expr, expected, lst_messages, msg in [
+@pytest.mark.parametrize(


I reformulated this test as a parametrized test because it was easier to debug...

mmatera · 2022-11-10T00:58:38Z

test/test_rules_patterns.py

 from .helper import check_evaluation


+DEBUGRULESPAT = int(os.environ.get("DEBUGRULESPAT", "0")) == 1


Still, there is a part of the behavior that I did not cover. This allows you to choose to mark the corresponding tests as xfail or skip, as in other unit tests.

mmatera · 2022-11-10T00:59:58Z

test/test_rules_patterns.py

+    ],
+)
+@skip_or_fail
+def test_one_identity_stil_failing(str_expr, str_expected, msg):


To pass these tests, we should fix the Rule.apply method. I left it for another round.

TiagoCavalcante

Looks great!

rocky · 2022-11-12T10:49:58Z

mathics/builtin/attributes.py

+    >> a /. f[u_] -> {u}
+     = a
+
+    Also, it does not affect evaluation:


I find these examples and the documentation to be extremely confusing.

Do you mind if I just rewrite the examples and doc here?

A small change is to replace "Also, it does not" with "'OneIdentity' does not"

Page 272 of the Mathematica 5 book motivates OneIdentity:

... in the case where x_ matches a single argument in a flat function, the question comes up as to whether the object it matches is really just and argument a itself or f[a]. Mathematica choses the first of these cases if the function carries the attribute OneIdentity and chooses the second case otherwise.

In/Out 16-20 of the book make this clear.

Sure, go ahead.

mathics/builtin/assignments/internals.py

rocky · 2022-11-12T11:10:42Z

test/test_rules_patterns.py

+        ),
+        (None, None, None),
+    ],
+)


Thanks for adding these tests. The ones from Patterns.html I think should go in a doctests with suitable commentary (and then they could be removed here); they were written with the intention of describing how OneIdentity works.

mathics/core/pattern.py

test/test_rules_patterns.py

mathics/core/pattern.py

rocky · 2022-11-12T13:28:45Z

@mmatera In trying to come up with examples, I am not seeing this follow the examples shown in https://reference.wolfram.com/language/tutorial/Patterns.html


In[1]:= SetAttributes[r, Flat]
Out[1]= None

In[2]:= r[a, b, b, c] /. r[x_, x_] -> rp[x]
Out[2]= r[a, b, b, c]  (* Wrong: should be r[a, rp[r[b]], c] *)

In[3]:= SetAttributes[r, OneIdentity]
Out[3]= None

In[4]:= r[a, b, b, c] /. r[x_, x_] -> rp[x]
Out[4]= r[a, b, b, c]  (* Wrong: should be r[a, rp[b], c] *)

Thoughts?

What is an example that shows the difference between Flat versus Flat and OneIdentity?

Also, I notice that SetAttributes[OneIdentity] on WMA should give the error:

SetAttributes::argr: 
   SetAttributes called with 1 argument; 2 arguments are expected.

mmatera · 2022-11-12T14:57:04Z

@mmatera In trying to come up with examples, I am not seeing this follow the examples shown in https://reference.wolfram.com/language/tutorial/Patterns.html
In[1]:= SetAttributes[r, Flat]
Out[1]= None

In[2]:= r[a, b, b, c] /. r[x_, x_] -> rp[x]
Out[2]= r[a, b, b, c]  (* Wrong: should be r[a, rp[r[b]], c] *)

In[3]:= SetAttributes[r, OneIdentity]
Out[3]= None

In[4]:= r[a, b, b, c] /. r[x_, x_] -> rp[x]
Out[4]= r[a, b, b, c]  (* Wrong: should be r[a, rp[b], c] *)
Thoughts?

What is an example that shows the difference between Flat versus Flat and OneIdentity?

Also, I notice that SetAttributes[OneIdentity] on WMA should give the error:
SetAttributes::argr: 
   SetAttributes called with 1 argument; 2 arguments are expected.

This is something that I detected, but I didn't try to fix yet. I have put a test for this marked as xfail

symbols.py: system_symbols() -> symbol_set(); "systems_symbols" name is too close Symbol( System` to module systemsymbols. Also, we now require symbols as parameters), not strings. systemsymbols.py: more system symbols

It appears this was originally Pattern.create. I suspect due to bad modularity and a lack of understandig Python that an import could be added inside the routine, this static method got moved outside of the class. Later on, the modularity was fixed, but the hack persisted. These kinds of code smells side effects of poor communication.

wehn it gets fixed.