Repository: sympy/sympy

Issue:
`Permutation` and `Cycle` as a callables have confusing behavior
There was some cleverness in the design of `Permutation` as a callable to mimic cycle notation.  However, I think this is really confusing.

Cycle notation:  when a permutation is written as a product of cycles, for example "(a,b)(c,d)" and you wish to apply it to a list `l`, that generally means

```
l[c], l[d] = l[d], l[c]
l[a], l[b] = l[b], l[a]
```

That is, first do the swap `(a,b)` then do the swap `(c,d)`.  The `Permutation` object cleverly uses callable to mimic this notation.  That is, you can construct an object `Permutation(a,b)(c,d)` since `Permutation(a,b)` is callable.  However, this seems contradictory to how functions are normally treated in a programming language.

When I encounter a function like `( Perm(swap a,b) )(swap c,d)` I expect that first `a,b` are swapped and then `c,d` are swapped which is equivalent in the current codebase to `Permutation(c,d)(a,b)`.  I think at minimum there needs to be a `kwarg` to allow the reversed order that I expect, but I also think that using callable this way is too clever a use of a programming grammar to look like a common mathematical notation.

Discussion:
smichr: ping @siefkenj 
siefkenj: I'm finding this pretty hard to follow...The functionality I need is the following:  A call `Permuation(some_list).list() == some_list` provided that `some_list` is a permutation and a call `Permutation(list_of_tuples).list()` applies the cycles from left-to-right.  Maybe I am misreading the code, but it seems like `Cycle(some_list)` applies `some_list` as a cycle, not as an explicitly specified permutation as `Permutation` would.
smichr: Here's a demo:

```
>>> from sympy.combinatorics import *
>>> Permutation([1,2,3,5,0,4]).list()
[1, 2, 3, 5, 0, 4]
>>> l2r = Permutation([1,2], [2,3])  # multi-lists is NEW
>>> r2l = Permutation([[1,2], [2,3]])  # list of lists (old R to L evaluation)
>>> l2r == Permutation(2,3)(1,2)
True
>>> r2l == Permutation(1,2)(2,3)
True
```

siefkenj: Okay.  I still don't think this can be used for my purposes, though.  If someone gives me a list of numbers, it is an explicit permutation, and if someone gives me a list of pairs, it is a list of swaps when evaluated left to right.  So, if someone gives me `w`, I cannot do `Permutation(w)`, nor can I do `Permutation(*w)`, which isn't to comment on whether this is a worthwhile addition to the `Permutation` class, but it doesn't free me from going a bunch of manual argument checking.
smichr: > but it doesn't free me from going a bunch of manual argument checking.

See point (2) in the OP. With that you could do `Permutation(w, from_left=True)` if the `from_left` were only used to handle the list of lists case. But I would prefer to just fix this via that pathway given in the second sentence of point 2. What do you think about that?

In the meanwhile, I think `Permutation(w if not w or not is_sequence(w[0]) else list(reversed(w)))` should work.
jksuom: It is often confusing that there are two different ways of composing permutations. This is how the product of permutations is defined in SymPy:
> Return the product a*b as a Permutation; the ith value is b(a(i)).

However, permutations are also mappings and, when considered as mappings, the composed mapping whose value at i is b(a(i)) is denoted b°a (with degree sign emulating the proper \circ sign).

Hence 'left to right' using one composition means 'right to left' with the other composition. So the ordering of cycles in this example
```
        >>> C((2, 3), (1, 2))
        (1 3 2)
        >>> C((1, 2), (2, 3))
        (1 2 3)
```
is opposite to that of composed permutations
```
>>> p = Permutation(Cycle(1, 2))
>>> q = Permutation(Cycle(2, 3))
>>> q*p  # ordered as in C((2, 3), (1, 2))
Permutation(1, 2, 3)
>>> p*q
Permutation(1, 3, 2)
```
jksuom: I'm afraid that having two methods of composing permutations would lead to confusion and coding errors. The definition currently used in SymPy is prevalent in the literature, and it is the one that those working in this area are expecting. That is also the order in which permutations are returned by some matrix methods. The alternative order does not seem to be used in SymPy. Hence I would prefer keeping a single order as in calls like `Cycle(a, b)(c, d)` (where a, b, c and d need not be all different).
smichr: 
> The definition currently used in SymPy is prevalent in the literature

Glad for that reassurance.

OK, then, perhaps just adding a docstring example of the `*` multiplication and the `Cycle` notation would suffice. 

```
>>> a = Cycle(1, 2)
>>> b = Cycle(2, 3)
>>> Permutation(a)*Permutation(b) == Cycle(1,2)(2,3)
True
```

In [this comment] I already demonstrate how the result for @siefkenj 's case can be handled.



asmeurer: > I'm afraid that having two methods of composing permutations would lead to confusion and coding errors. The definition currently used in SymPy is prevalent in the literature, and it is the one that those working in this area are expecting. That is also the order in which permutations are returned by some matrix methods. The alternative order does not seem to be used in SymPy. Hence I would prefer keeping a single order as in calls like Cycle(a, b)(c, d) (where a, b, c and d need not be all different).

I completely agree with this. 
smichr: I will simply add a note in the docstring about the ordering issue and that it only matters when the cycles are not disjoint. Or maybe the docs would be a better place for this?

Here is a workaround for @siefkenj :+1: 
```
>>> def LRPerm(w):
...     from sympy.utilities.iterables import is_sequence
...     return Permutation(w if not (w and is_sequence(w[0])) else
...     Permutation.rmul(*[Permutation(*i) for i in w]))
>>> LRPerm(((1,2),(2,3)))  # cycle input
(123)
>>> Permutation(2,3)(1,2)
(123)
>>> LRPerm((1,2,0))  # actual-permutation input
(012)
```
smichr: If someone can see if this added note is clear, then this is done.

Patch:
diff --git a/doc/src/tutorial/calculus.rst b/doc/src/tutorial/calculus.rst
index fcfa4a6e0f..1c2c3c3365 100644
--- a/doc/src/tutorial/calculus.rst
+++ b/doc/src/tutorial/calculus.rst
@@ -323,15 +323,13 @@ the ``differentiate_finite`` function:
 
     >>> f, g = symbols('f g', cls=Function)
     >>> differentiate_finite(f(x)*g(x))
-    -f(x - 1/2)⋅g(x - 1/2) + f(x + 1/2)⋅g(x + 1/2)
-
-If we want to expand the intermediate derivative we may pass the
-flag ``evaluate=True``:
-
-    >>> differentiate_finite(f(x)*g(x), evaluate=True)
     (-f(x - 1/2) + f(x + 1/2))⋅g(x) + (-g(x - 1/2) + g(x + 1/2))⋅f(x)
 
-This form however does not respect the product rule.
+If we don't want to expand the intermediate derivative we may pass the
+flag ``evaluate=False``:
+
+    >>> differentiate_finite(f(x)*g(x), evaluate=False)
+    -f(x - 1/2)⋅g(x - 1/2) + f(x + 1/2)⋅g(x + 1/2)
 
 If you already have a ``Derivative`` instance, you can use the
 ``as_finite_difference`` method to generate approximations of the
diff --git a/sympy/calculus/finite_diff.py b/sympy/calculus/finite_diff.py
index 0371be76a1..0c7fa606ef 100644
--- a/sympy/calculus/finite_diff.py
+++ b/sympy/calculus/finite_diff.py
@@ -430,8 +430,8 @@ def differentiate_finite(expr, *symbols,
     wrt: Symbol, optional
         see ``Derivative.as_finite_difference``
     evaluate : bool
-        kwarg passed on to ``diff``, whether or not to
-        evaluate the Derivative intermediately (default: ``False``).
+        kwarg passed on to ``diff`` (whether or not to
+        evaluate the Derivative intermediately).
 
 
     Examples
@@ -439,33 +439,26 @@ def differentiate_finite(expr, *symbols,
 
     >>> from sympy import cos, sin, Function, differentiate_finite
     >>> from sympy.abc import x, y, h
-    >>> f, g = Function('f'), Function('g')
-    >>> differentiate_finite(f(x)*g(x), x, points=[x-h, x+h])
-    -f(-h + x)*g(-h + x)/(2*h) + f(h + x)*g(h + x)/(2*h)
-
-    Note that the above form preserves the product rule in discrete form.
-    If we want we can pass ``evaluate=True`` to get another form (which is
-    usually not what we want):
-
-    >>> differentiate_finite(f(x)*g(x), x, points=[x-h, x+h], evaluate=True).simplify()
-    -((f(-h + x) - f(h + x))*g(x) + (g(-h + x) - g(h + x))*f(x))/(2*h)
-
-    ``differentiate_finite`` works on any expression:
-
+    >>> f = Function('f')
     >>> differentiate_finite(f(x) + sin(x), x, 2)
-    -2*f(x) + f(x - 1) + f(x + 1) - 2*sin(x) + sin(x - 1) + sin(x + 1)
-    >>> differentiate_finite(f(x) + sin(x), x, 2, evaluate=True)
     -2*f(x) + f(x - 1) + f(x + 1) - sin(x)
+    >>> differentiate_finite(f(x) + sin(x), x, 2, evaluate=False)
+    -2*f(x) + f(x - 1) + f(x + 1) - 2*sin(x) + sin(x - 1) + sin(x + 1)
     >>> differentiate_finite(f(x, y), x, y)
     f(x - 1/2, y - 1/2) - f(x - 1/2, y + 1/2) - f(x + 1/2, y - 1/2) + \
 f(x + 1/2, y + 1/2)
+    >>> g = Function('g')
+    >>> differentiate_finite(f(x)*g(x), x, points=[x-h, x+h]).simplify()
+    -((f(-h + x) - f(h + x))*g(x) + (g(-h + x) - g(h + x))*f(x))/(2*h)
+    >>> differentiate_finite(f(x)*g(x), x, points=[x-h, x+h], evaluate=False)
+    -f(-h + x)*g(-h + x)/(2*h) + f(h + x)*g(h + x)/(2*h)
 
     """
     # Key-word only arguments only available in Python 3
     points = kwargs.pop('points', 1)
     x0 = kwargs.pop('x0', None)
     wrt = kwargs.pop('wrt', None)
-    evaluate = kwargs.pop('evaluate', False)
+    evaluate = kwargs.pop('evaluate', True)
     if kwargs != {}:
         raise ValueError("Unknown kwargs: %s" % kwargs)
 
diff --git a/sympy/combinatorics/permutations.py b/sympy/combinatorics/permutations.py
index 104f7616e7..af08ac7e00 100644
--- a/sympy/combinatorics/permutations.py
+++ b/sympy/combinatorics/permutations.py
@@ -540,8 +540,8 @@ class Permutation(Basic):
     >>> Permutation(1,2,3) == Permutation(2,3,1) == Permutation(3,1,2)
     True
 
-    The disjoint cycle notation is convenient when representing permutations
-    that have several cycles in them:
+    The disjoint cycle notation is convenient when representing
+    permutations that have several cycles in them:
 
     >>> Permutation(1, 2)(3, 5) == Permutation([[1, 2], [3, 5]])
     True
@@ -554,6 +554,29 @@ class Permutation(Basic):
     >>> _ == Permutation([[1, 2]])*Permutation([[1, 3]])*Permutation([[2, 3]])
     True
 
+        Caution: when the cycles have common elements
+        between them then the order in which the
+        permutations are applied matters. The
+        convention is that the permutations are
+        applied from *right to left*. In the following, the
+        transposition of elements 2 and 3 is followed
+        by the transposition of elements 1 and 2:
+
+        >>> Permutation(1, 2)(2, 3) == Permutation([(1, 2), (2, 3)])
+        True
+        >>> Permutation(1, 2)(2, 3).list()
+        [0, 3, 1, 2]
+
+        If the first and second elements had been
+        swapped first, followed by the swapping of the second
+        and third, the result would have been [0, 2, 3, 1].
+        If, for some reason, you want to apply the cycles
+        in the order they are entered, you can simply reverse
+        the order of cycles:
+
+        >>> Permutation([(1, 2), (2, 3)][::-1]).list()
+        [0, 2, 3, 1]
+
     Entering a singleton in a permutation is a way to indicate the size of the
     permutation. The ``size`` keyword can also be used.
 


Test patch:
diff --git a/sympy/calculus/tests/test_finite_diff.py b/sympy/calculus/tests/test_finite_diff.py
index b70ea5fa15..a75b92927f 100644
--- a/sympy/calculus/tests/test_finite_diff.py
+++ b/sympy/calculus/tests/test_finite_diff.py
@@ -114,17 +114,18 @@ def test_as_finite_diff():
 def test_differentiate_finite():
     x, y = symbols('x y')
     f = Function('f')
-    res0 = differentiate_finite(f(x, y) + exp(42), x, y, evaluate=True)
+    res0 = differentiate_finite(f(x, y) + exp(42), x, y)
     xm, xp, ym, yp = [v + sign*S(1)/2 for v, sign in product([x, y], [-1, 1])]
     ref0 = f(xm, ym) + f(xp, yp) - f(xm, yp) - f(xp, ym)
     assert (res0 - ref0).simplify() == 0
 
     g = Function('g')
-    res1 = differentiate_finite(f(x)*g(x) + 42, x, evaluate=True)
+    res1 = differentiate_finite(f(x)*g(x) + 42, x)
     ref1 = (-f(x - S(1)/2) + f(x + S(1)/2))*g(x) + \
            (-g(x - S(1)/2) + g(x + S(1)/2))*f(x)
     assert (res1 - ref1).simplify() == 0
 
-    res2 = differentiate_finite(f(x) + x**3 + 42, x, points=[x-1, x+1])
+    res2 = differentiate_finite(f(x) + x**3 + 42, x, points=[x-1, x+1],
+                                evaluate=False)
     ref2 = (f(x + 1) + (x + 1)**3 - f(x - 1) - (x - 1)**3)/2
     assert (res2 - ref2).simplify() == 0