make symbolic series subclass of Expression

[rwst]

Making Expression.series create SymbolicSeries (a subclass of Expression) and moving the series code into a separate file series.pyx

• reflects the fact that expressions and series don't commute
• allows upcoming fixes that don't clutter Expression methods with if ex.is_a_series():
• makes for better documentation via having a Symbolic Series ref man page

In refactoring language this is "replace conditional with polymorphism".

#16203, #17400, and #17402 depend on this.

Depends on #19948

Component: symbolics

Author: Ralf Stephan

Branch/Commit: 1832f4a

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/17659

[rwst]

Branch: u/rws/17659

[rwst]

Commit: a4d2084

[rwst]

New commits:

a4d2084

17659: make symbolic series subclass of Expression

[nbruin]

comment:3

I have nothing invested in how symbolics and series interact, but I see some immediate problems that would arise from subclassing Expression for a Series type:

• when does a SymbolicSeries expression get created? The problem is that the SymbolicRing by itself is a rather untyped environment, so how do you know that an object needs to be made in this specialized class
• How do SymbolicSeries in different variables interact, e.g.:

F = (1/(x+1))*y+sin(x)*y^2+O(y^3)
G = (y/(y^2-2)*x + cos(y)*x^2 +O(x^3)
F+G

Is the result a series? If so, in which variable(s)? One solution would be a SymbolicSeriesRing as a parent, which declares in what variables the series are. I suspect this would be nearly useless for whatever problem the current design tries to solve, though.

[rwst]

comment:4

Nils, this is not about creating a ring but simple refactoring. No behaviour change.

http://www.refactoring.com/catalog/replaceConditionalWithPolymorphism.html

Replying to @nbruin:

I have nothing invested in how symbolics and series interact, but I see some immediate problems that would arise from subclassing Expression for a Series type:

These problems already exist, so nothing is new.

[sagetrac-git]

Changed commit from a4d2084 to ed003f0

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

ed003f0

16203: make imports more specific to prevent import loops

[nbruin]

comment:8

OK, I thought that f.is_series() would do some non-trivial logic to see if f can be used as a series, but it's just exposing a flag that is held internally somewhere. You could indeed expose that information in the type instead, but with ducktyping, subtyping and sage's parent infrastructure doing so might not be as convenient as it might be in systems that are really governed by explicit types. You'd need to think if this refactoring actually will help for further development.

Indeed, being a "series" seems a rather fickle property. It doesn't seem to be preserved under anything:

sage: var('x,y');
sage: f=cos(y).series(y,10)
sage: g=sin(x).series(x,10)
sage: var('x,y');
sage: f=cos(y).series(y,10)
sage: g=sin(x).series(x,10)
sage: f.is_series()
True
sage: (f+f).is_series()
False
sage: (-f).is_series()
False
sage: (f+g).series(x,10)
Order(1)

The last result probably follows from interpreting the Order(x^10) term in g as an order term in y, and hence equivalent to Order(y^0).

[rwst]

comment:9

Replying to @nbruin:

You'd need to think if this refactoring actually will help for further development.

It isolates the code and the documentation.

Indeed, being a "series" seems a rather fickle property. It doesn't seem to be preserved under anything:

There are functions in pynac-0.3.2/ginac/pseries.cpp to compare, add, multiply (also with constant), power (to constant). My plan is to only provide conversion to and from PowerSeries (#10846, #16203, #17402) and fix the worst bugs (#17400, #16213) and that's it. I'd rather improve PowerSeries but symbolic is what people use.

[sagetrac-git]

Changed commit from ed003f0 to f97d47d

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

02885af	Merge branch 'develop' into t/17659/17659
f97d47d	17659: add SymbolicSeries.default_variable() and coefficients()

[rwst]

Changed branch from u/rws/17659 to public/17659

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

3198603

17659: factor out SymbolicSeries from Expression

[sagetrac-git]

Changed commit from f97d47d to 3198603

[sagetrac-git]

Changed commit from 3198603 to a67d0ce

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

a67d0ce

Merge branch 'develop' into t/17659/public/17659

[rwst]

comment:16

There is a spurious unreprodcible doctest failure in the 6.6beta2 pachbot run.

[sagetrac-git]

Changed commit from a67d0ce to cb7bbed

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7183f4d	Merge branch 'develop' into t/17659/public/17659
cb7bbed	17659: replace PY_NEW

[rwst]

comment:18

This now uncovers a regression of #12255 which was not really fixed.

File "src/sage/symbolic/expression.pyx", line 5279, in sage.symbolic.expression.Expression.coefficients
Failed example:
    f.coefficients(g)
Exception raised:
    ValueError: The name "g(t)" is not a valid Python identifier.

I will now let this ticket (and with it its dependencies) lie in limbo because, as a posting on sage-devel showed, there is no interest in symbolic series.

[rwst]

Dependencies: #19448

[rwst]

Changed dependencies from #19448 to #19948

[vbraun]

comment:46

sage -t --long src/sage/symbolic/series.pyx
**********************************************************************
File "src/sage/symbolic/series.pyx", line 71, in sage.symbolic.series
Failed example:
    x*ex1
Expected:
    (1*x + (-1/6)*x^3 + Order(x^4))*x
Got:
    x*(1*x + (-1/6)*x^3 + Order(x^4))
**********************************************************************
1 item had failures:
   1 of  23 in sage.symbolic.series
    [42 tests, 1 failure, 0.04 s]

[rwst]

comment:47

Beware the dependency!

[vbraun]

comment:49

Patchbot also shows failure

[rwst]

Changed branch from u/rws/17659-1 to u/rws/17659-2

[rwst]

comment:51

Yes, a remainder from the time before #19948.

---

New commits:

c3be6a1	Merge branch 'u/rws/17659-1' of trac.sagemath.org:sage into tmp04
1832f4a	17659: fix doctest

[rwst]

Changed commit from d5cc424 to 1832f4a

[tscrim]

comment:53

Ralf, did you intend for this to wait to 7.2 or did you mean 7.1?

[vbraun]

Changed branch from u/rws/17659-2 to 1832f4a