Opened 8 years ago
Closed 8 years ago
#14870 closed defect (fixed)
Failure with Python int modulo a rational
Reported by: | tscrim | Owned by: | tscrim |
---|---|---|---|
Priority: | critical | Milestone: | sage-5.12 |
Component: | basic arithmetic | Keywords: | |
Cc: | vbraun, roed | Merged in: | sage-5.12.beta0 |
Authors: | Travis Scrimshaw | Reviewers: | Beth Malmskog |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Some fun I came across:
sage: int(5) % QQ(2) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-4-8ec0f9994ded> in <module>() ----> 1 int(Integer(5)) % QQ(Integer(2)) TypeError: Argument 'self' has incorrect type (expected sage.rings.rational.Rational, got int)
Note that this works:
sage: 5 % QQ(2) 1
Attachments (1)
Change History (11)
comment:1 Changed 8 years ago by
- Cc vbruan roed added
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
- Cc vbraun added; vbruan removed
comment:3 Changed 8 years ago by
You shouldn't check the type exactly, derived classes of rational should be treated like a rational.
Also, the whole PY_*
macro usage that you see in the sources is mostly historical, nowadays Cython understands isinstance(x, Rational)
just fine and this is the preferred form of a type check.
comment:4 Changed 8 years ago by
- Status changed from needs_review to positive_review
We tried using decimals like 3.0, coerced rationals like QQ(3), non-reduced fractions like 4/2, complex-like values like 3+0*I, and absolute values of integers as moduli (second argument in function)--all of these were correctly interpreted as integers. We also found correct error messages for moduli that didn't make sense, for example mod(31/10, 10) produces a correct error message. Looks good!
Changed 8 years ago by
comment:5 Changed 8 years ago by
New version as per Volker's comment.
comment:6 Changed 8 years ago by
@malmskog, you'll need to write your real name as the reviewer.
comment:7 Changed 8 years ago by
- Reviewers set to Beth Malmskog
comment:8 Changed 8 years ago by
- Milestone changed from sage-5.12 to sage-5.11
comment:9 Changed 8 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:10 Changed 8 years ago by
- Merged in set to sage-5.12.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
Fixed. Thanks Volker and David for your insight.