Ticket #9481 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

random_element fails for power series over real field, has inaccurate docstring

Reported by: niles Owned by: malb
Priority: major Milestone: sage-4.6.2
Component: commutative algebra Keywords: power series, random element, beginner
Cc: rishi Work issues:
Report Upstream: N/A Reviewers: Aly Deines
Authors: Niles Johnson, Jeroen Demeyer Merged in: sage-4.6.2.alpha1
Dependencies: Stopgaps:

Description

The random_element method of univariate power series does not pass arguments to the underlying polynomial ring accurately, and the description of its second argument is inaccurate.

c.f. this  thread from sage-devel 

sage: SQ = PowerSeriesRing(QQ,'v')
sage: SR = PowerSeriesRing(RR,'v')

sage: SQ.random_element(5,100)  # docstring promises coefficients are uniformly distributed between -100 and 100
-7/3 + 5/8*v + 37/60*v^2 + 33/8*v^3 + 77/89*v^4 + O(v^5)

sage: SR.random_element(5)  # broken
Traceback (most recent call last):
...
TypeError: unsupported operand type(s) for -: 'int' and 'NoneType'

Attachments

trac_9481_ps_random_element.patch Download (4.3 KB) - added by niles 3 years ago.
emulated behavior of polynomial ring random_element, as suggested on sage-devel; commit message now references ticket number
trac_9481_ps_random_element.2.patch Download (4.3 KB) - added by jdemeyer 2 years ago.
Same patch, fixed commit message
9481_docstring.patch Download (779 bytes) - added by jdemeyer 2 years ago.
Apply on top of previous patch

Change History

comment:1 Changed 3 years ago by niles

  • Priority changed from minor to major
  • Status changed from new to needs_review

Changed 3 years ago by niles

emulated behavior of polynomial ring random_element, as suggested on sage-devel; commit message now references ticket number

comment:2 Changed 3 years ago by rishi

  • Cc rishi added

comment:3 Changed 2 years ago by niles

  • Keywords element, beginner added; element removed
  • Authors changed from niles to Niles Johnson

comment:4 Changed 2 years ago by aly.deines

  • Status changed from needs_review to positive_review
  • Reviewers set to Aly Deines

Looks good.

comment:5 Changed 2 years ago by jdemeyer

  • Milestone changed from sage-4.6.1 to sage-4.6.2

Changed 2 years ago by jdemeyer

Same patch, fixed commit message

comment:6 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Docstring needs reformatting to proper Sphinx markup.

Changed 2 years ago by jdemeyer

Apply on top of previous patch

comment:7 Changed 2 years ago by jdemeyer

  • Status changed from needs_work to needs_review
  • Authors changed from Niles Johnson to Niles Johnson, Jeroen Demeyer

comment:8 Changed 2 years ago by niles

  • Status changed from needs_review to positive_review

All documentation now builds without error or warning, so positive review for 9481_docstring.patch Download

comment:9 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.6.2.alpha1
Note: See TracTickets for help on using tickets.