Opened 11 years ago

Closed 10 years ago

#9306 closed enhancement (fixed)

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: Merged in: sage-4.7.alpha4
Authors: Simon Spicer Reviewers: Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges


Consider the following:

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

This is fine so far. However:

sage: floor(q)
sage: ceil(q)
sage: round(q)

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

Attachments (1)

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

Download all attachments as: .zip

Change History (4)

Changed 10 years ago by spice

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

comment:1 Changed 10 years ago by spice

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

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 10 years ago by kini

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

Five doctests failed, then failed to fail when I retested them, including devel/sage/sage/tests/ . 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 10 years ago by jdemeyer

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