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

functions PowerSeries.ogf() and egf() named wrong #15705

Closed
rwst opened this issue Jan 22, 2014 · 20 comments
Closed

functions PowerSeries.ogf() and egf() named wrong #15705

rwst opened this issue Jan 22, 2014 · 20 comments

Comments

@rwst
Copy link

rwst commented Jan 22, 2014

The functions PowerSeries.ogf() and egf() are named wrong

The documentation states: Returns the ordinary generating function associated to self. But the function is a wrapper for the Pari function serlaplace() which actually converts to ordinary g.f. in the case of an exponential g.f.

Example: 1+x+x<sup>2+x</sup>3+x<sup>4+O(x</sup>5) is generated both by 1/(1-x)+O(x^5) or itself, but:

sage: R.<x> = PowerSeriesRing(ZZ)
sage: (1+x+x^2+x^3+x^4+O(x^5)).ogf()
1 + x + 2*x^2 + 6*x^3 + 24*x^4

which is clearly wrong given name and definition.

So, I hope you agree it's necessary if I rename ogf() to egf_to_ogf() and egf() to ogf_to_egf() and adapt the docs.

Component: combinatorics

Keywords: series ogf

Author: Ralf Stephan

Branch/Commit: 813a807

Reviewer: Nathann Cohen

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

@rwst rwst added this to the sage-6.1 milestone Jan 22, 2014
@rwst rwst self-assigned this Jan 22, 2014
@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jan 22, 2014

Branch: u/rws/ticket/15705

@rwst
Copy link
Author

rwst commented Jan 22, 2014

New commits:

4b79c25Trac #15705: PowerSeries.ogf() and .egf(): match name and definition to behaviour

@rwst
Copy link
Author

rwst commented Jan 22, 2014

Commit: 4b79c25

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Feb 21, 2014

comment:6

It appears that we would have to deprecate the old names first.

@rwst rwst changed the title function PowerSeries.ogf() named wrong functions PowerSeries.ogf() and egf() named wrong Feb 21, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2014

Changed commit from 4b79c25 to 85b5f9b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2014

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

4f8c7beMerge branch 'develop' into ticket/15705
bbc5196Merge branch 'develop' into ticket/15705
85b5f9badd back ogf() and egf() with deprecation warning; fix doctests

@rwst
Copy link
Author

rwst commented Mar 7, 2014

Changed author from rws to Ralf Stephan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2014

Changed commit from 85b5f9b to 019861a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 12, 2014

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

019861aMerge branch 'develop' (6.2.beta4) into ticket/15705

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 25, 2014

comment:10

Hellooooooooo !

You should probably use deprecated_function_alias.

http://www.sagemath.org/doc/developer/coding_in_python.html#deprecation

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2014

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

daaf616Merge branch 'develop' into rev/15705
4ae59beuse deprecated_function_alias() instead

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2014

Changed commit from 019861a to 4ae59be

@rwst
Copy link
Author

rwst commented Mar 25, 2014

comment:12

You're right!

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 25, 2014

Changed branch from u/rws/ticket/15705 to public/15705

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 25, 2014

Changed commit from 4ae59be to 813a807

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 25, 2014

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Mar 25, 2014

comment:13

Hmmmmm... at first I did not like the new names at all, and prefered .to_egf() and .to_ogf(), but the way you did it will make it the easiest to find for new users... Hmmmm.... !

I just added a small commit on top of yours to fix two things:

  1. The ogf=... and egf=... had a wrong indentation level

  2. The deprecated_function_alias function was not imported

All tests pass. If you agree with my changes you can set the ticket to positive_review.

Nathann


New commits:

813a807trac #15705: reviewer's patch

@vbraun
Copy link
Member

vbraun commented Mar 31, 2014

Changed branch from public/15705 to 813a807

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

2 participants