Custom sorting key for substitute( )

[Upabjojr]

Substitute now accepts custom sorting key to specify custom sorting for elements in Multiset.

This should solve the compatibility problem with SymPy which does not allow comparison operators (>, >=, <, <=) to be evaluated to booleans if the truth of the expression cannot be easily determined.

[coveralls]

Coverage increased (+0.004%) to 96.389% when pulling 6829e16623009649842ce4443a76dcd28d608d01 on Upabjojr:substitute_custom_sorting_key into 5cae3f275e3a1f725518516bf3c700dc3be03a56 on HPAC:master.

[wheerd]

I think adding some global state is not a good idea, I would prefer changing the signature of the substitute function:

def substitute(expression: Union[Expression, Pattern], substitution: Substitution, sort_key=None: Optional[Callable[[Expression], Any]]) -> Replacement:
    ...

That way the behavior of the function remains pure and does not depend on some global state.

[wheerd]

@Upabjojr I switch the build from Travis CI to Github Actions. If you rebase your changes, the pipeline should work better :)

[Upabjojr]

I think adding some global state is not a good idea, I would prefer changing the signature of the substitute function

I am not very convinced about adding a parameter to substitute, because you would need to add that parameter every time you call substitute with a SymPy expression.

Ideally after calling from sympy.utilities.matchpy_connector import *, everything in MatchPy should work fine with SymPy expressions. A global parameter makes it easier. The alternative would be a wrapper for substitute( ) in SymPy.

Otherwise, would about editing the Multiset library instead and add the sorting key there?

[wheerd]

What if you put a substitute function in sympy.utilities.matchpy_connector that calls the matchpy substitute with the correct sort_key?

[Upabjojr]

What if you put a substitute function in sympy.utilities.matchpy_connector that calls the matchpy substitute with the correct sort_key?

Well, that would solve the problem if users import substitute( ) correctly. I fear there will still be some confusion about users mixing up the imports. I was still trying to avoid duplicating the definition of substitute( ).

Notice that redefining substitute( ) inside of SymPy could be done without modifying MatchPy. For example, the multiset could be pre-sorted and passed as a list to MatchPy.

[wheerd]

Notice that redefining substitute( ) inside of SymPy could be done without modifying MatchPy. For example, the multiset could be pre-sorted and passed as a list to MatchPy.

Isn't that a better approach then anyways?

Relying on and modifying a private variable from outside is against Python conventions and seems like a very brittle solution. An alternative would be to have setter/getter for that global state, even though I would still prefer to make it explicit when calling the function..

[wheerd]

Thank you for your contribution! 🎉👍