Opened 7 years ago

Closed 7 years ago

#15705 closed defect (fixed)

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

Reported by: rws Owned by: rws
Priority: major Milestone: sage-6.2
Component: combinatorics Keywords: series ogf
Cc: Merged in:
Authors: Ralf Stephan Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: 813a807 (Commits, GitHub, GitLab) Commit: 813a8075202e9e6c70e83f4fafc0e7af7ef66d4b
Dependencies: Stopgaps:

Status badges

Description (last modified by rws)

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^2+x^3+x^4+O(x^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.

Change History (15)

comment:1 Changed 7 years ago by rws

  • Description modified (diff)

comment:2 Changed 7 years ago by rws

  • Branch set to u/rws/ticket/15705
  • Created changed from 01/22/14 10:01:58 to 01/22/14 10:01:58
  • Modified changed from 01/22/14 10:05:53 to 01/22/14 10:05:53

comment:3 Changed 7 years ago by rws

  • Commit set to 4b79c254541220c8bb1877a314bf8287edc6f360
  • Status changed from new to needs_review

New commits:

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

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:5 Changed 7 years ago by rws

  • Status changed from needs_review to needs_work

comment:6 Changed 7 years ago by rws

  • Description modified (diff)
  • Summary changed from function PowerSeries.ogf() named wrong to functions PowerSeries.ogf() and egf() named wrong

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

comment:7 Changed 7 years ago by git

  • Commit changed from 4b79c254541220c8bb1877a314bf8287edc6f360 to 85b5f9b91ca4e096d23399c983130ec099badcdc

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

comment:8 Changed 7 years ago by rws

  • Authors changed from rws to Ralf Stephan
  • Status changed from needs_work to needs_review

comment:9 Changed 7 years ago by git

  • Commit changed from 85b5f9b91ca4e096d23399c983130ec099badcdc to 019861aec5fb1f59940d2524c14f00f05105a146

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

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

comment:10 Changed 7 years ago by ncohen

  • Status changed from needs_review to needs_work

Hellooooooooo !

You should probably use deprecated_function_alias.

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

Nathann

comment:11 Changed 7 years ago by git

  • Commit changed from 019861aec5fb1f59940d2524c14f00f05105a146 to 4ae59bea6a7e7ab45b9839ee1cd05ff2f70b9317

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

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

comment:12 Changed 7 years ago by rws

  • Status changed from needs_work to needs_review

You're right!

comment:13 Changed 7 years ago by ncohen

  • Branch changed from u/rws/ticket/15705 to public/15705
  • Commit changed from 4ae59bea6a7e7ab45b9839ee1cd05ff2f70b9317 to 813a8075202e9e6c70e83f4fafc0e7af7ef66d4b
  • Reviewers set to Nathann Cohen

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

comment:14 Changed 7 years ago by rws

  • Status changed from needs_review to positive_review

comment:15 Changed 7 years ago by vbraun

  • Branch changed from public/15705 to 813a8075202e9e6c70e83f4fafc0e7af7ef66d4b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.