Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Type checks in arith.py #14858

Closed
eviatarbach opened this issue Jul 5, 2013 · 7 comments
Closed

Type checks in arith.py #14858

eviatarbach opened this issue Jul 5, 2013 · 7 comments
Assignees
@aghitza
Labels
c: basic arithmetic p: critical / 2 t: bug
Milestone
sage-6.1

Comments

@eviatarbach
Copy link

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.

Component: basic arithmetic

Author: Eviatar Bach

Branch: u/vbraun/arith_in_ZZ

Reviewer: Nathann Cohen

Issue created by migration from https://trac.sagemath.org/ticket/14858
@eviatarbach eviatarbach added this to the sage-6.1 milestone Jul 5, 2013
@eviatarbach
Copy link
Author

Attachment: trac14858.py.gz

@eviatarbach
Copy link
Author

comment:1

Attachment: trac14858.patch.gz

Oops, accidentally used the .py extension at first.

Patchbot apply trac14858.patch

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 26, 2013

comment:2

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

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 26, 2013

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 26, 2013

Author: Eviatar Bach

@eviatarbach
Copy link
Author

comment:3

Thank you for reviewing!

Eviatar

@vbraun
Copy link
Member

vbraun commented Jan 4, 2014

Branch: u/vbraun/arith_in_ZZ

@vbraun vbraun closed this as completed Jan 5, 2014
tscrim pushed a commit to tscrim/sage that referenced this issue Jun 1, 2023
Merge branch 'develop' into public/algebras/fp_algebras
f2d515a 
* develop: (95 commits)
  Trac sagemath#14858: more robust type checks for arith.py
  Updated Sage version to 6.1.beta3
  Trac sagemath#11271: making the curve inaccessible + documentation
  Trac sagemath#11271: Corrections to is_surjective in Galois representations over Q.
  sage-sdist: copy upstream tarballs using sage-spkg
  Fixed docbuild.
  Removed removed file from doc.
  Fix wrong NOTE block.
  Where possible, remove optional - database_stein_watkins
  Stein-Watkins database: reviewer patch
  buid/deps : added a dependency of freetype on libpng in order to work around a build-time race condition.
  Replace bytes_to_long/long_to_bytes by ord/chr.
  Revert "Filter pycrypto warning about insecure modular exponentiaiton."
  first version of the git-trac package
  Filter pycrypto warning about insecure modular exponentiaiton.
  Update PyCrypto to version 2.6.1.
  Let PyCrypto build on FreeBSD.
  trac sagemath#15435: WeylGroup and CoxeterGroup to groups.<tab>
  trac sagemath#15369: Alias groups.misc.AdditiveCyclic to IntegerModRing
  Fix for comparison of padics.
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aghitza aghitza

Labels
c: basic arithmetic p: critical / 2 t: bug
Projects
None yet
Milestone
sage-6.1
Development

No branches or pull requests
3 participants
@aghitza @eviatarbach @vbraun