Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overridable node visitors instead of typed-nodes in expression trees #75

Open
Upabjojr opened this issue Nov 20, 2023 · 2 comments
Open

Comments

@Upabjojr
Copy link
Contributor

The current MatchPy implementation defines expression trees and allows expression trees to be overloaded by external library (as it is done in SymPy's connector to MatchPy). Unfortunately, the current MatchPy implementation relies on types and isinstance( ) calls to determine the expression node.
For example:

  • isinstance(label, Wildcard) to check if a label is a wildcard,
  • isinstance(expression, Operation)
  • isinstance(subpattern, CommutativeOperation)
  • isinstance(expression, AssociativeOperation)
  • isinstance(subpattern, OneIdentityOperation)

The first one is quite tricky in SymPy because it needs to define a new class that is both a MatchPy and SymPy object.

There are other approaches to expression tree that do not defined typed nodes (i.e. classes that represent a particular type of node). For example, protosym has a unique class to define many types of nodes.

Does it make sense to create an overridable method is check whether a node is a wildcard/operation/associative operation/commutative operation/etc... so that custom libraries may define their way to visit their expression trees?

@wheerd
Copy link
Collaborator

wheerd commented Nov 21, 2023

For operations, matchpy uses the ABCMeta metaclass which allows registering any type as a subclass: https://docs.python.org/3/library/abc.html#abc.ABCMeta.register
I don't know if there is a need for doing the same for wildcards, maybe it is enough to create a subclass that is both a sympy class and a subclass of Wildcard.
I would rather not do a big change to how expression trees function in matchpy.

@Upabjojr
Copy link
Contributor Author

For operations, matchpy uses the ABCMeta metaclass which allows registering any type as a subclass:

That's the limitation, there are many examples of libraries using a single class for all expression trees. I'm not sure you can handle those cases unless you really hack/override python's typing methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants