Ticket #9306 (closed enhancement: fixed)

Opened 3 years ago

Last modified 2 years ago

round incoherent with ceil/floor on rational numbers

Reported by: zimmerma Owned by: AlexGhitza
Priority: minor Milestone: sage-4.7
Component: basic arithmetic Keywords: round
Cc: Work issues:
Report Upstream: N/A Reviewers: Keshav Kini
Authors: Simon Spicer Merged in: sage-4.7.alpha4
Dependencies: Stopgaps:

Description

Consider the following: 

sage: q=2^200/3^50
sage: q.floor()
2238393297946874000179418290327143433
sage: q.ceil()
2238393297946874000179418290327143434
sage: q.round()
2238393297946874000179418290327143433

This is fine so far. However: 

sage: floor(q)
2238393297946874000179418290327143433
sage: ceil(q)
2238393297946874000179418290327143434
sage: round(q)
2.23839329795e+36

We would expect round(q) to behave like q.round().

Attachments

trac_9306_round_on_rationals.patch Download (2.3 KB) - added by spice 2 years ago.
Changes the round() command to defer to an element's .round() method if no precision is specified.

Change History

Changed 2 years ago by spice

Changes the round() command to defer to an element's .round() method if no precision is specified.

comment:1 Changed 2 years ago by spice

  • Keywords round added
  • Status changed from new to needs_review
  • Authors set to Simon Spicer

The above change alters the behaviour of sage's round() command. Before it *always* returned a real double field element; now it defers to an element's .round() method if no precision is specified, i.e. a sage Integer is returned in these cases. This makes round(x) and x.round() agree whenever x has a .round() method.

comment:2 Changed 2 years ago by kini

  • Status changed from needs_review to positive_review
  • Reviewers set to Keshav Kini

Five doctests failed, then failed to fail when I retested them, including devel/sage/sage/tests/startup.py . Code is easy to read and clearly does what it is intended to do, which intent I agree with. Everything looks good!

comment:3 Changed 2 years ago by jdemeyer

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