Split mathics builtin patterns #1123
Conversation
|
@mmatera have the changes quiesced to the point that I should pull this branch and try it? |
|
@rocky, sure. This is just what it is. |
|
Let's follow the organization in the docs. So Rules and Patterns is called Patterns and under than are Basic Pattern Objects and Composite Patterns, etc. |
|
In the guide of the link, rules are not included, except as a link to other guide named "Rules & Patterns" . But ok, let's follow this organization as far as it is possible. |
Yeah, I realized things are neither clear cut nor optimal. Possibly Rules would be in its own section as a sibling of Patterns. Following the existing organization does have the advantage that -- within a given section like Rules, or Pattern Matching Functions -- it is usually easy to see what is covered and what is missing, (for those functions that have a strong affinity to only one section which does occur quite a bit.) |
|
Much better - thanks! Some additional things that would make this closer to the current style.
|
mathics/builtin/patterns/rules.py
Outdated
|
|
||
|
|
||
| # TODO: disentangle me | ||
| def create_rules( |
There was a problem hiding this comment.
This is really an eval function and should be moved to mathics.eval.rule and probably called eval_Replace().
|
I think now this is ready. |
mathics/builtin/patterns/rules.py
Outdated
| >> Replace[x[x[y]], x -> z, All] | ||
| = x[x[y]] | ||
|
|
||
| Heads can be replaced using the Heads option |
mathics/builtin/patterns/rules.py
Outdated
| >> Replace[x[x[y]], x -> z, {1}, Heads -> True] | ||
| = z[x[y]] | ||
|
|
||
| You can use Replace as an operator |
mathics/builtin/patterns/rules.py
Outdated
| """ | ||
|
|
||
| messages = { | ||
| "reps": "`1` is not a valid replacement rule.", |
There was a problem hiding this comment.
This line can be deleted if should have been "rep". And then the message call below in 255 should be adjusted.
There was a problem hiding this comment.
BTW I haven't been able to check this since I can't get to galecia
There was a problem hiding this comment.
BTW I haven't been able to check this since I can't get to galecia
Yes, it seems there is a power outage in the building where galatea is located.
mathics/builtin/patterns/rules.py
Outdated
| def eval_levelspec(self, expr, rules, ls, evaluation, options): | ||
| "Replace[expr_, rules_, Optional[Pattern[ls, _?LevelQ], {0}], OptionsPattern[Replace]]" | ||
| try: | ||
| rules, ret = create_rules(rules, expr, "Replace", evaluation) |
There was a problem hiding this comment.
create_rules -> eval_Replace in mathics.eval.pattern
mathics/builtin/patterns/rules.py
Outdated
| from mathics.eval.rules import Dispatch, create_rules | ||
|
|
||
| # This tells documentation how to sort this module | ||
| sort_order = "mathics.builtin.rules-and-patterns.rules" |
There was a problem hiding this comment.
sort order no longer matches title def...
|
It is definitely much better! And closer to our current practice. There are still a number of small changes. If you want me to make them, I will do. |
|
@rocky , I have implemented almost all your suggestions. Fell free to add the changes you consider useful. |
| the rules are applied. | ||
|
|
||
| >> dispatchrule = Dispatch[{rule}]; | ||
| >> a + F[F[x ^ 2]] //. dispatchrule |
There was a problem hiding this comment.
It looks like this test triggers a bug in the code!
There was a problem hiding this comment.
Yep. Hopefully, it is fixed in 07d1768. However, probably we need to go over Dispatch and create_rules again.
|
Well, finally all that got broken, and what we discovered was broken seems to work now. |
See review comments form `plit_mathics_builtin_patterns`. Additionally, it looks like `sort_order` for sections has been fixed. Co-authored-by: Juan Mauricio Matera <[email protected]>
|
LGTM now. I see more corrects that should go in. But will do that in due time. |
Just split this long module into smaller parts.