Opened 10 years ago
Closed 8 years ago
#14766 closed defect (fixed)
Fix Python int problem with exp_integral
Reported by: | kcrisman | Owned by: | burcin |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | symbolics | Keywords: | |
Cc: | benjaminfjones, burcin, eviatarbach | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Ralf Stephan |
Report Upstream: | N/A | Work issues: | |
Branch: | 68c545a (Commits, GitHub, GitLab) | Commit: | 68c545a5edb07ea87adea5850a143195723dfced |
Dependencies: | #17130 | Stopgaps: |
Description
In #11143 we weren't careful about Python ints. There are lots of examples of this, so one will want to go through the whole file.
sage: exp_integral_e(int(3),0) 0 sage: exp_integral_e(3,0) 1/2
Change History (11)
comment:1 Changed 10 years ago by
Type: | PLEASE CHANGE → defect |
---|
comment:2 Changed 9 years ago by
Cc: | benjaminfjones,burcin, → benjaminfjones, burcin |
---|---|
Milestone: | sage-5.11 → sage-5.12 |
comment:3 Changed 9 years ago by
Cc: | eviatarbach added |
---|
comment:4 Changed 9 years ago by
I think my preference would be either raising a TypeError
(and clearly documenting what the allowed types are) or doing Python 3 style division. I don't like the idea of adding an implicit coercion int
-> Integer
.
If we do type checking in BuiltinFunction.__call__
, we should make sure that there isn't a significant performance penalty.
comment:5 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:6 Changed 9 years ago by
Milestone: | sage-6.2 → sage-6.3 |
---|
comment:7 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
comment:8 Changed 8 years ago by
Branch: | → u/jdemeyer/ticket/14766 |
---|---|
Created: | Jun 18, 2013, 5:37:43 PM → Jun 18, 2013, 5:37:43 PM |
Modified: | Aug 10, 2014, 4:51:03 PM → Aug 10, 2014, 4:51:03 PM |
comment:9 Changed 8 years ago by
Authors: | → Jeroen Demeyer |
---|---|
Commit: | → 68c545a5edb07ea87adea5850a143195723dfced |
Dependencies: | → #17130 |
Status: | new → needs_review |
New commits:
6d10729 | Simplify numerical evaluation of BuiltinFunctions
|
b6e1ed4 | Merge remote-tracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130
|
382f97a | Merge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1
|
7265989 | 17130: reviewer's patch: fix typo
|
c47dbd4 | Fix Trac #17328 again in a better way
|
a486db2 | Call the factorial() method of an Integer
|
68c545a | Fix bugs due to Python 2 int division
|
comment:10 Changed 8 years ago by
Reviewers: | → Ralf Stephan |
---|---|
Status: | needs_review → positive_review |
Looks fine and passes tests in functions/
.
comment:11 Changed 8 years ago by
Branch: | u/jdemeyer/ticket/14766 → 68c545a5edb07ea87adea5850a143195723dfced |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Note: See
TracTickets for help on using
tickets.
I'm guessing this occurs in other files too.
I think it's unreasonable to expect everyone adding symbolic functions to worry about this, and certainly users to be aware of it; maybe we could make
BuiltinFunction.__call__
automatically convertint
toInteger
? Or have it raise an error if anint
is in the parameters? Allowingint
into_eval_
causes other problems too, such as getting expressions withint
pyobjects in them, making arithmetic slower, etc.At the very least, we could temporarily add
from __future__ import division
in all files that implement symbolic functions; then exact answers wouldn't be returned in all cases where they should, but at least mathematically wrong answers wouldn't occur.