Opened 7 years ago

Closed 6 years ago

#14858 closed defect (fixed)

Type checks in arith.py

Reported by: eviatarbach Owned by: AlexGhitza
Priority: critical Milestone: sage-6.1
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Eviatar Bach Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: u/vbraun/arith_in_ZZ (Commits) Commit:
Dependencies: Stopgaps:

Description

Many type checks in rings/arith.py use isinstance for checking whether something is an integer, for example. They should use x in ZZ instead, because not all cases are covered, leading to errors like the following: 

sage: rising_factorial(-4, 2)
12
sage: rising_factorial(-4, SR(2))
0
sage: rising_factorial(SR(-4), SR(2))
RuntimeError: indeterminate expression: 0 * infinity encountered.

Setting to critical because this is very confusing, and can give silent errors such as rising_factorial(-4, 2) being different from rising_factorial(-4, SR(2)). The fact that rising_factorial(-4, SR(2)) gives 0 is a separate error, ticket #14857.

Attachments (2)

trac14858.py​ (2.5 KB) - added by eviatarbach 7 years ago.
trac14858.patch​ (2.5 KB) - added by eviatarbach 7 years ago.

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by eviatarbach

Changed 7 years ago by eviatarbach

comment:1 Changed 7 years ago by eviatarbach

  • Status changed from new to needs_review

Oops, accidentally used the .py extension at first.

Patchbot apply trac14858.patch

comment:2 Changed 6 years ago by ncohen

  • Authors set to Eviatar Bach
  • Reviewers set to Nathann Cohen
  • Status changed from needs_review to positive_review

Patch applies, the patchbot says it's good, the patch looks good... Thanks !

Nathann

comment:3 Changed 6 years ago by eviatarbach

Thank you for reviewing!

Eviatar

comment:4 Changed 6 years ago by vbraun

  • Branch set to u/vbraun/arith_in_ZZ

comment:5 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.