Opened 11 years ago
Closed 6 years ago
#9248 closed defect (fixed)
docstring for factorial doesn't say that it accepts non-integer, non-symbolic input
Reported by: | ddrake | Owned by: | ddrake |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
At comment:2:ticket:9240, we see someone get confused about the docstring for factorial
, which claims it takes as input an integer or symbolic expression. However, it takes non-integer, non-SR inputs:
sage: x = 1.5; parent(x) Real Field with 53 bits of precision sage: factorial(x) 1.32934038817914 sage: x = 3/2; parent(x) Rational Field sage: factorial(x) 3/4*sqrt(pi) sage: x = CC(1+I); parent(x) Complex Field with 53 bits of precision sage: factorial(x) 0.652965496420167 + 0.343065839816545*I
I understand that there is coercion going on, but we should specify that the function takes pretty much any complex number (except of course negative integers) and evaluates (something akin to) gamma(1+x).
However, it doesn't exactly do gamma(1+x):
sage: x = I; parent(x) Symbolic Ring sage: factorial(x) 0.498015668118356 - 0.154949828301811*I sage: gamma(x+1) gamma(I + 1) sage: parent(factorial(x)) Symbolic Ring sage: gamma(x+1).n() 0.498015668118356 - 0.154949828301811*I sage: parent(gamma(x+1).n()) Complex Field with 53 bits of precision
The factorial function clearly is not simply calling gamma(x+1) when x is not an integer.
Change History (14)
comment:1 follow-up: ↓ 8 Changed 11 years ago by
- Owner changed from mvngu to ddrake
comment:2 follow-up: ↓ 3 Changed 10 years ago by
Notice also that factorial does not accept all sorts of input, which leads to problems in this thread.
comment:3 in reply to: ↑ 2 Changed 10 years ago by
Replying to kcrisman:
Notice also that factorial does not accept all sorts of input, which leads to problems in this thread.
Which for some reason I still posted even after realizing this is just #9240. Sorry for the noise.
comment:4 Changed 7 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:5 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:6 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:7 Changed 6 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:8 in reply to: ↑ 1 Changed 6 years ago by
comment:9 Changed 6 years ago by
- Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
It appears the originally described issue was fixed in #12286 (misleading title).
comment:10 Changed 6 years ago by
- Status changed from needs_review to positive_review
please change the status of a ticket to 'positive review' when you flag it as wontfix.
comment:11 follow-up: ↓ 13 Changed 6 years ago by
The same person should not both set it to wontfix and give it a positive review as it skips the review process.
comment:12 Changed 6 years ago by
On the other hand, closing a ticket without review is much less bad than merging a branch without review. And essentially nobody reviews sage-duplicate/invalid/wontfix tickets.
comment:13 in reply to: ↑ 11 Changed 6 years ago by
The same person should not both set it to wontfix and give it a positive review as it skips the review process.
But the invalid and duplicate I think this is okay for. I often give a positive review to dups or obviously now-functioning tickets.
And essentially nobody reviews sage-duplicate/invalid/wontfix tickets.
Yeah, probably also true.
I do have to say that perhaps we should have a separate wontfix that would require two reviewers, maybe? I have occasionally set some notebook tickets to wontfix but that is a fairly unusual situation.
comment:14 Changed 6 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
Also, factorial doesn't seem to accept the algorithm keyword anymore, even though the docstring says it does! I see that factorial() is just a wrapper around GiNaC's factorial; how does GiNaC compute factorials?