Mathics3 / mathics-core

An open-source Mathematica. This repository contains the Python modules for WL Built-in functions, variables, core primitives, e.g. Symbol, a parser to create Expressions, and an evaluator to execute them.
https://mathics.org
Other
786 stars 48 forks source link

Earlier initialization of patterns #1103

Closed mmatera closed 2 months ago

mmatera commented 2 months ago

When a ExpressionPattern is created, the way in which the matching with expressions is determined in part by the attributes of its head. For example, if S is Orderless, the match method of the pattern S[a_, v__Integer] should check for different orders of the arguments.

Also, the match against patterns like DirectedInfinity[1] can be determined using the faster sameQ method, instead of using all the machinery for looking for named patterns inside.

This PR introduces some modifications in the way Pattern objects are created, to have earlier access to the attributes, and to determine if the pattern is a "Literal"

rocky commented 2 months ago

Please let me know when it should be looked at on a cursory level. Basically, there was already Literal work planned and there was also a total revamp of pattern matching. The Go code that was mentioned elsewhere should be looked at and considered.

The little I looked at looked more like underbrush removal which we find ourselves doing too often. But some consideration should be given to how this might fit into the bigger picture of planned work. I don't think this conflicts, but I'd like to think about it and check and/or discuss it.

mmatera commented 2 months ago

Please let me know when it should be looked at on a cursory level. Basically, there was already Literal work planned and there was also a total revamp of pattern matching.

Actually, here it is not so much: just make PatternBase.create more aware about the attributes of the pattern it has to create, and if the pattern is just a "Literal" ("Verbatim"?) expression, just tag the Pattern object to use sameQ instead of something more sophisticated that tries to track named patterns, or Orderless expressions.

The question here is if it makes sense to add specific classes for these "Literal" patterns, or just keep a single ExpressionPattern class, that changes its methods according to the attributes and tags.

rocky commented 2 months ago

Actually, here it is not so much: just make PatternBase.create more aware about the attributes of the pattern it has to create, and if the pattern is just a "Literal" ("Verbatim"?) expression, just tag the Pattern object to use sameQ instead of something more sophisticated that tries to track named patterns, or Orderless expressions.

Ah ok. I just pulled the branch and looked it over thinking about things. This is not disruptive. Adding a property or attribute indicating whether a pattern is literal is good. And being able to use SameQ to test pattern equality is also good.

I am sorry for the delay in getting to this.

The question here is if it makes sense to add specific classes for these "Literal" patterns, or just keep a single ExpressionPattern class, that changes its methods according to the attributes and tags.

Right now, I don't see a need for a separate class like there is for ListExpression.

I am fine with merging this in now as is.

mmatera commented 2 months ago

Actually, here it is not so much: just make PatternBase.create more aware about the attributes of the pattern it has to create, and if the pattern is just a "Literal" ("Verbatim"?) expression, just tag the Pattern object to use sameQ instead of something more sophisticated that tries to track named patterns, or Orderless expressions.

Ah ok. I just pulled the branch and looked it over thinking about things. This is not disruptive. Adding a property or attribute indicating whether a pattern is literal is good. And being able to use SameQ to test pattern equality is also good.

I am sorry for the delay in getting to this.

The question here is if it makes sense to add specific classes for these "Literal" patterns, or just keep a single ExpressionPattern class, that changes its methods according to the attributes and tags.

Right now, I don't see a need for a separate class like there is for ListExpression.

I am fine with merging this in now as is.

OK, in that case, let's merge and iterate