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

Refactoring in Chebyshev functions #24554

Open
rwst opened this issue Jan 17, 2018 · 18 comments
Open

Refactoring in Chebyshev functions #24554

rwst opened this issue Jan 17, 2018 · 18 comments

Comments

@rwst
Copy link

rwst commented Jan 17, 2018

While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply BuiltinFunctions or GinacFunctions this ticket suffers from the fact that the Cheby functions in Sage allow the algorithm keyword (by using a __call__ method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function chebyshev_T()) dispatching on the keyword, especially because doctests involving all kind of Sage objects as function argument are expected to work eternally.

The goal of this ticket is to move the __call__ method out of the Function class (as well as other methods used as interface) into a separate interface class; to 2. make the remaining class inherit from BuiltinFunction and, 3., to remove the ChebyshevFunction superclass as it is then no longer used.

Component: symbolics

Author: Ralf Stephan

Branch/Commit: u/rws/24554 @ 957ce14

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

@rwst rwst added this to the sage-8.2 milestone Jan 17, 2018
@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jan 18, 2018

comment:2

The usage chebyshev_T(...algorithm='pari' silently uses normal evaluation because there is no _eval_pari_ member, so Pari was never supported.

@rwst

This comment has been minimized.

@rwst rwst changed the title Remove __call__() usage in Chebyshev functions Refactoring in Chebyshev functions Jan 18, 2018
@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jan 20, 2018

comment:5

Also the usage chebyshev_T(...algorithm='maxima') used normal evaluation. Maxima returns a polynomial in (x-1) which probably is useful so I'm changing doctests. In a later step this could be renamed to a more descriptive name and computed by Sage.

@rwst
Copy link
Author

rwst commented Jan 20, 2018

comment:6

Also eval_formula() should become algorithm='hypergeometric'. The Maxima "algorithm" is variant of it, see https://en.wikipedia.org/wiki/Chebyshev_polynomials#Explicit_expressions

@rwst
Copy link
Author

rwst commented Jan 20, 2018

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 21, 2018

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

23190a624554: refactor chebyshev_U
258233024554: remove ChebyshevFunction

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 21, 2018

Commit: 2582330

@rwst
Copy link
Author

rwst commented Jan 21, 2018

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Mar 3, 2018

comment:10

Docs fail to build, doctest errors in three files:

  • sage -t --long src/sage/rings/polynomial/polynomial_element.pyx # 6 doctests failed
  • sage -t --long src/sage/rings/number_field/splitting_field.py # 2 doctests failed
  • sage -t --long src/sage/graphs/bipartite_graph.py # 1 doctest failed

@rwst
Copy link
Author

rwst commented Mar 3, 2018

@rwst
Copy link
Author

rwst commented Mar 3, 2018

New commits:

957ce1424554: Refactoring in Chebyshev functions

@rwst
Copy link
Author

rwst commented Mar 3, 2018

Changed commit from 2582330 to 957ce14

@videlec
Copy link
Contributor

videlec commented Feb 22, 2020

comment:13

merge failure

@mkoeppe
Copy link
Member

mkoeppe commented Apr 14, 2020

comment:15

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Sep 5, 2020
@mkoeppe
Copy link
Member

mkoeppe commented Feb 13, 2021

comment:17

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe
Copy link
Member

mkoeppe commented Jul 19, 2021

comment:18

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants