Repository: sympy/sympy
Pull request: Created class for finite fourier series
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
fixes #15701 follows #15979 
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Following #15979 the FiniteFourierSeries class is improved to handle finite fourier series while keeping the API equivalent to FourierSeries class.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* series
  * Improves fourier.FiniteFourierSeries class
<!-- END RELEASE NOTES -->


Problem statement:
Fourier Series of sin(x)
Obviously the Fourier series of sin(x) is sin(x) but
Try the following:
```
from sympy import Function, sin, fourier_series, pi
from sympy.abc import x

f = Function('f')(x)
f = sin(x)

s = fourier_series(f) # or e.g: fourier_series(f, (x,-pi, pi))
s.truncate(2) # it works for n = 1 but for 2 the statement doesn't respond
```
*P.S*: I am using sympy 1.3.

Hints from issue discussion:
The problem seems to be [this](https://github.com/sympy/sympy/blob/master/sympy/series/fourier.py#L189-L194) in `FourierSeries.truncate`:
```
    terms = []
    for t in self:
        if len(terms) == n:
            break
        if t is not S.Zero:
            terms.append(t)
```
Because the Fourier series itself works fine:
```
>>> s = fourier_series(sin(x))
>>> it = iter(s)
>>> [next(it) for _ in range(5)]
[0, sin(x), 0, 0, 0]
```

The problem is that passing `n=2` to `fourier_series` wants to take _2 nonzero_ orders from the truncated Fourier series. This implies that in case of a finite series we end up in an infinite loop if we request more terms than the number of nonvanishing orders in the finite series.

So the question is how should this be fixed? I'd naively think that `truncate` truncates with respect to orders of the frequency rather than number of nonvanishing terms. But this would mean a very different behaviour compared to what we currently have:
```
>>> s = fourier_series(sin(x) + cos(x)**3)
>>> [next(it) for _ in range(5)]
[0, sin(x) + 3*cos(x)/4, 0, cos(3*x)/4, 0]
>>> s.truncate(1)
sin(x) + 3*cos(x)/4
>>> s.truncate(2)
sin(x) + 3*cos(x)/4 + cos(3*x)/4
```
I'd expect `s.truncate(n=2)` to give me terms up to `cos(2*x)`/`sin(2*x)`, which would fix the infinite loop (instead of counting nonzero terms we could just take `n` orders and ignore the vanishing ones afterward). But this would not be backward-compatible. The current behaviour is [even documented](https://docs.sympy.org/latest/modules/series/fourier.html#sympy.series.fourier.FourierSeries.truncate).

Is there any (robust) way to determine the number of terms in a `FourierSeries` when their number is finite? I suspect comparing the truncated series to `s.function` is both needlessly expensive and very fragile...
I had added a ValueError such that if n in truncate is greater than the non zero terms in fourier series such that the sum of non zero terms becomes equal to the given function. Moreover an appropriate error message is also shown with the number of non zero terms in the respected fourier series
@s1291 please read our [code of conduct](https://github.com/sympy/sympy/blob/master/CODE_OF_CONDUCT.md). Your comment is in violation of it. Please keep your comments constructive without name calling. 
Here is how I would suggest to fix this. We need to check if the input function has a finite Fourier series. A finite Fourier series is a finite linear combination of sines and cosines with arguments `n*x`. Probably applying the appropriate trig expansion functions from the `fu` module, and checking if it looks like a Fourier series should be sufficient. If it is, the FourierSeries object should instead return a subclass FiniteFourierSeries, which operates directly on the finite series. 
@asmeurer Ohh okk okk I would like to work upon this issue and would like to do changes as you suggested.
@jmig5776 are you still working on it?
Yes I m working on it .
@asmeurer I have started working on this according to your suggestion.

Commit message: added docstrings for clarity

Patch to review:
diff --git a/sympy/series/fourier.py b/sympy/series/fourier.py
index 04dacd3b8771..72fe639f3c71 100644
--- a/sympy/series/fourier.py
+++ b/sympy/series/fourier.py
@@ -96,19 +96,17 @@ def finite_check(f, x, L):
     def check_fx(exprs, x):
         return x not in exprs.free_symbols
 
-    def check_sincos(expr, x, L):
-        if type(expr) == sin or type(expr) == cos:
-            sincos_args = expr.args[0]
+    def check_sincos(_expr, x, L):
+        if type(_expr) == sin or type(_expr) == cos:
+            sincos_args = _expr.args[0]
 
             if sincos_args.match(a*(pi/L)*x + b) is not None:
                 return True
             else:
                 return False
 
-    expr = sincos_to_sum(TR2(TR1(f)))
-    res_expr = S.Zero
-    add_coeff = expr.as_coeff_add()
-    res_expr += add_coeff[0]
+    _expr = sincos_to_sum(TR2(TR1(f)))
+    add_coeff = _expr.as_coeff_add()
 
     a = Wild('a', properties=[lambda k: k.is_Integer, lambda k: k != S.Zero, ])
     b = Wild('b', properties=[lambda k: x not in k.free_symbols or k == S.Zero, ])
@@ -118,8 +116,8 @@ def check_sincos(expr, x, L):
         for t in mul_coeffs:
             if not (check_fx(t, x) or check_sincos(t, x, L)):
                 return False, f
-        res_expr += TR10(s)
-    return True, res_expr.collect([sin(a*(pi/L)*x), cos(a*(pi/L)*x)])
+
+    return True, _expr
 
 
 class FourierSeries(SeriesBase):
@@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
+            bn is a dictionary of coefficients of sin terms
+             `bn[k] = coefficient of sin(pi*(k/L)*x)`
+
+            or exprs can be an expression to be converted to fourier form
+
+       Methods
+       =======
+       This class is an extension of FourierSeries class.
+       Please refer to sympy.series.fourier.FourierSeries for
+       further information.
+
+       See Also
+       ========
+
+       sympy.series.fourier.FourierSeries
+       sympy.series.fourier.fourier_series
+       """
+
+    def __new__(cls, f, limits, exprs):
+
+        if not (type(exprs) == tuple and len(exprs) == 3):  # exprs is not of form (a0, an, bn)
+
+            # Converts the expression to fourier form
+            exprs = exprs.as_coeff_add()
+            exprs = exprs[0] + Add(*[TR10(i) for i in exprs[1]])
+            exp_ls = exprs.expand(trig=False, power_base=False, power_exp=False, log=False).as_coeff_add()
+
+            a0 = exp_ls[0]
+            x = limits[0]
+            L = abs(limits[2] - limits[1]) / 2
+
+            a = Wild('a', properties=[lambda k: k.is_Integer, lambda k: k != S.Zero, ])
+            b = Wild('b', properties=[lambda k: x not in k.free_symbols or k == S.Zero, ])
+
+            an = dict()
+            bn = dict()
+
+            # separates the coefficients of sin and cos terms in dictionaries an, and bn
+            for p in exp_ls[1]:
+                t = p.match(b * cos(a * (pi / L) * x))
+                q = p.match(b * sin(a * (pi / L) * x))
+                if t:
+                    an[t[a]] = t[b] + an.get(t[a], S.Zero)
+                elif q:
+                    bn[q[a]] = q[b] + bn.get(q[a], S.Zero)
+                else:
+                    a0 += p
 
+            exprs = (a0, an, bn)
 
-def fourier_series(f, limits=None):
+        args = map(sympify, (f, limits, exprs))
+
+        return Expr.__new__(cls, *args)
+
+    @property
+    def interval(self):
+        _length = 1 if self.a0 else 0
+        _length += max(set(self.an.keys()).union(set(self.bn.keys()))) + 1
+        return Interval(0, _length)
+
+    @property
+    def length(self):
+        return self.stop - self.start
+
+    @property
+    def L(self):
+        return abs(self.period[1] - self.period[0]) / 2
+
+    def truncate(self, n=oo):
+        """
+        Return the first n nonzero terms of the series if n is less than the size of the finite fourier series.
+
+        If n is None return an iterator.
+
+        See Also
+        ========
+
+        sympy.series.fourier.FourierSeries.truncate
+        """
+        if n is None:
+            return iter(self)
+
+        terms = []
+        for t in self:
+            if len(terms) == n:
+                break
+            if t is not S.Zero:
+                terms.append(t)
+
+        return Add(*terms)
+
+    def shiftx(self, s):
+        """Shift x by a term independent of x.
+
+        f(x) -> f(x + s)
+
+        This is fast, if Fourier series of f(x) is already
+        computed.
+
+        See Also
+        ========
+        sympy.series.fourier.FourierSeries.shiftx
+
+        """
+        s, x = sympify(s), self.x
+
+        if x in s.free_symbols:
+            raise ValueError("'%s' should be independent of %s" % (s, x))
+
+        _expr = self.truncate().subs(x, x + s)
+        sfunc = self.function.subs(x, x + s)
+
+        return self.func(sfunc, self.args[1], _expr)
+
+    def scale(self, s):
+        """Scale the function by a term independent of x.
+
+        f(x) -> s * f(x)
+
+        This is fast, if Fourier series of f(x) is already
+        computed.
+
+        See Also
+        ========
+        sympy.series.fourier.FourierSeries.scale
+
+        """
+        s, x = sympify(s), self.x
+
+        if x in s.free_symbols:
+            raise ValueError("'%s' should be independent of %s" % (s, x))
+
+        _expr = self.truncate() * s
+        sfunc = self.function * s
+
+        return self.func(sfunc, self.args[1], _expr)
+
+    def scalex(self, s):
+        """Scale x by a term independent of x.
+
+        f(x) -> f(s*x)
+
+        This is fast, if Fourier series of f(x) is already
+        computed.
+
+        See Also
+        ========
+        sympy.series.fourier.FourierSeries.scalex
+        """
+        s, x = sympify(s), self.x
+
+        if x in s.free_symbols:
+            raise ValueError("'%s' should be independent of %s" % (s, x))
+
+        _expr = self.truncate().subs(x, x * s)
+        sfunc = self.function.subs(x, x * s)
+
+        return self.func(sfunc, self.args[1], _expr)
+
+    def _eval_term(self, pt):
+        if pt == 0:
+            return self.a0
+
+        _term = self.an.get(pt, S.Zero) * cos(pt * (pi / self.L) * self.x) \
+                + self.bn.get(pt, S.Zero) * sin(pt * (pi / self.L) * self.x)
+        return _term
+
+    def __add__(self, other):
+        if isinstance(other, FourierSeries):
+            return other.__add__(fourier_series(self.function, self.args[1],\
+                                                finite=False))
+        elif isinstance(other, FiniteFourierSeries):
+            if self.period != other.period:
+                raise ValueError("Both the series should have same periods")
+
+            x, y = self.x, other.x
+            function = self.function + other.function.subs(y, x)
+
+            if self.x not in function.free_symbols:
+                return function
+
+            return fourier_series(function, limits=self.args[1])
+
+    def __sub__(self, other):
+        return self.__add__(-other)
+
+
+def fourier_series(f, limits=None, finite=True):
     """Computes Fourier sine/cosine series expansion.
 
     Returns a :class:`FourierSeries` object.
@@ -511,11 +706,11 @@ def fourier_series(f, limits=None):
     if x not in f.free_symbols:
         return f
 
-    L = abs(limits[2] - limits[1])/2
-    is_finite, res_f = finite_check(f, x, L)
-
-    if is_finite:
-        return FiniteFourierSeries(res_f)
+    if finite:
+        L = abs(limits[2] - limits[1]) / 2
+        is_finite, res_f = finite_check(f, x, L)
+        if is_finite:
+            return FiniteFourierSeries(f, limits, res_f)
 
     n = Dummy('n')
     neg_f = f.subs(x, -x)


Code review comments:
diff_hunk: @@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
+            bn is a dictionary of coefficients of sin terms
+             `bn[k] = coefficient of sin(pi*(k/L)*x)`
+
+            or exprs can be an expression to be converted to fourier form
+
+       Methods
+       =======
+       This class is an extension of FourierSeries class.
+       Please refer to sympy.series.fourier.FourierSeries for
+       further information.
+
+       See Also
+       ========
+
+       sympy.series.fourier.FourierSeries
+       sympy.series.fourier.fourier_series
+       """
+
+    def __new__(cls, f, limits, exprs):
+
+        if not (type(exprs) == tuple and len(exprs) == 3):  # exprs is not of form (a0, an, bn)
+
+            # Converts the expression to fourier form
+            exprs = exprs.as_coeff_add()
path: sympy/series/fourier.py
text: @user1:
How about having something like `c, e` on the left hand side.

@author:
Yeah, it would be more readable but I didn't want to add unnecessary variables. I don't think it would be necessary.

@user1:
I think that the bytecode will be simpler with a local variable. Using a few more slots on the local parameter stack should not be problem.

@author:
I didn't do because this eventually is a single statement and the variables used are not going far. I can change that if you suggest.

diff_hunk: @@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
+            bn is a dictionary of coefficients of sin terms
+             `bn[k] = coefficient of sin(pi*(k/L)*x)`
+
+            or exprs can be an expression to be converted to fourier form
+
+       Methods
+       =======
+       This class is an extension of FourierSeries class.
+       Please refer to sympy.series.fourier.FourierSeries for
+       further information.
+
+       See Also
+       ========
+
+       sympy.series.fourier.FourierSeries
+       sympy.series.fourier.fourier_series
+       """
+
+    def __new__(cls, f, limits, exprs):
+
+        if not (type(exprs) == tuple and len(exprs) == 3):  # exprs is not of form (a0, an, bn)
+
+            # Converts the expression to fourier form
+            exprs = exprs.as_coeff_add()
+            exprs = exprs[0] + Add(*[TR10(i) for i in exprs[1]])
+            exp_ls = exprs.expand(trig=False, power_base=False, power_exp=False, log=False).as_coeff_add()
+
+            a0 = exp_ls[0]
+            x = limits[0]
+            L = abs(limits[2] - limits[1]) / 2
+
+            a = Wild('a', properties=[lambda k: k.is_Integer, lambda k: k != S.Zero, ])
+            b = Wild('b', properties=[lambda k: x not in k.free_symbols or k == S.Zero, ])
path: sympy/series/fourier.py
text: @user1:
Is `or k == S.Zero` necessary? `k = S.Zero` has no free symbols.

@author:
I also think it is unnecessary.

diff_hunk: @@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
+            bn is a dictionary of coefficients of sin terms
+             `bn[k] = coefficient of sin(pi*(k/L)*x)`
+
+            or exprs can be an expression to be converted to fourier form
+
+       Methods
+       =======
+       This class is an extension of FourierSeries class.
+       Please refer to sympy.series.fourier.FourierSeries for
+       further information.
+
+       See Also
+       ========
+
+       sympy.series.fourier.FourierSeries
+       sympy.series.fourier.fourier_series
+       """
+
+    def __new__(cls, f, limits, exprs):
+
+        if not (type(exprs) == tuple and len(exprs) == 3):  # exprs is not of form (a0, an, bn)
+
+            # Converts the expression to fourier form
+            exprs = exprs.as_coeff_add()
+            exprs = exprs[0] + Add(*[TR10(i) for i in exprs[1]])
+            exp_ls = exprs.expand(trig=False, power_base=False, power_exp=False, log=False).as_coeff_add()
+
+            a0 = exp_ls[0]
+            x = limits[0]
+            L = abs(limits[2] - limits[1]) / 2
+
+            a = Wild('a', properties=[lambda k: k.is_Integer, lambda k: k != S.Zero, ])
path: sympy/series/fourier.py
text: @user1:
There could be `k is not S.Zero` as `S.Zero` is a singleton. It will save a few function calls.

@author:
I don't know but k can be `0` at some time. But `S.Zero == 0` while `S.Zero is not 0`

@user1:
SymPy 0 is `S.Zero`. Can `k` be a non-Sympy number?

@author:
I guess you are right. Since anything reaching to that part of code has been sympified, `0` would eventually be `S.Zero`. Should I change it? This would initiate another build for tests. If there is anything more you want to suggest I would add it in this commit only to save some build time.

@user1:
I think you could change it. `S.Zero` was designed to make quick tests for equality possible. Maybe you could wait for some time after a new commit. It would be possible to push several commits at a time. I have not had time to finish reviewing yet.

diff_hunk: @@ -96,19 +96,17 @@ def finite_check(f, x, L):
     def check_fx(exprs, x):
         return x not in exprs.free_symbols
 
-    def check_sincos(expr, x, L):
-        if type(expr) == sin or type(expr) == cos:
-            sincos_args = expr.args[0]
+    def check_sincos(_expr, x, L):
+        if type(_expr) == sin or type(_expr) == cos:
path: sympy/series/fourier.py
text: @user1:
This could probably be `if isinstance(_expr, (sin, cos))`.

@author:
I can update it.

diff_hunk: @@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
+            bn is a dictionary of coefficients of sin terms
+             `bn[k] = coefficient of sin(pi*(k/L)*x)`
+
+            or exprs can be an expression to be converted to fourier form
+
+       Methods
+       =======
+       This class is an extension of FourierSeries class.
+       Please refer to sympy.series.fourier.FourierSeries for
+       further information.
+
+       See Also
+       ========
+
+       sympy.series.fourier.FourierSeries
+       sympy.series.fourier.fourier_series
+       """
+
+    def __new__(cls, f, limits, exprs):
+
+        if not (type(exprs) == tuple and len(exprs) == 3):  # exprs is not of form (a0, an, bn)
+
+            # Converts the expression to fourier form
+            exprs = exprs.as_coeff_add()
+            exprs = exprs[0] + Add(*[TR10(i) for i in exprs[1]])
+            exp_ls = exprs.expand(trig=False, power_base=False, power_exp=False, log=False).as_coeff_add()
path: sympy/series/fourier.py
text: @user1:
Here could be `a0, exp_ls` on the left.

@user1:
`a0` is defined anyway.

@author:
I think I should update this as well.

diff_hunk: @@ -440,16 +438,213 @@ def __sub__(self, other):
         return self.__add__(-other)
 
 
-class FiniteFourierSeries(Basic):
-    def __new__(cls, *args):
-        obj = Basic.__new__(cls, *args)
-        return obj
+class FiniteFourierSeries(FourierSeries):
+    r"""Represents Finite Fourier sine/cosine series.
 
-    def truncate(self, n=3):
-        return Add(*self._args)
+       For how to compute Fourier series, see the :func:`fourier_series`
+       docstring.
+
+       Parameters
+       ==========
+       f : Expr
+            Expression for finding fourier_series
+
+       limits : ( x, start, stop)
+            x is the independent variable for the expression f
+            (start, stop) is the period of the fourier series
+
+       exprs: (a0, an, bn) or Expr
+            a0 is the constant term a0 of the fourier series
+            an is a dictionary of coefficients of cos terms
+             `an[k] = coefficient of cos(pi*(k/L)*x)`
path: sympy/series/fourier.py
text: @user1:
There could be double backquotes as LaTeX is not needed.

@author:
I will update.