Opened 5 years ago
Closed 21 months ago
#20861 closed defect (duplicate)
numbers.Rational, numbers.Integer, etc. interfaces not implemented correctly
Reported by: | dgulotta | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-duplicate/invalid/wontfix |
Component: | basic arithmetic | Keywords: | rational, numerator, denominator |
Cc: | jdemeyer, mcbell, slelievre | Merged in: | |
Authors: | Reviewers: | Samuel Lelièvre | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The file numbers_abc.py registers some Sage classes as instances of numbers.Rational, numbers.Integer, etc. However, none of these classes implement the interfaces entirely correctly, because members that the interface expects to be properties (e. g. numerator and denominator in numbers.Rational) are instead functions.
Attempts to use these interfaces will fail in a way that may be confusing to the user:
sage: import fractions sage: fractions.Fraction(1/2) Fraction(<built-in method numerator of sage.rings.rational.Rational object at 0x7f4268425b30>, <built-in method denominator of sage.rings.rational.Rational object at 0x7f4268425b30>) sage: fractions.Fraction(1,2) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-3-cee53c78dd93> in <module>() ----> 1 fractions.Fraction(Integer(1),Integer(2)) /home/dan/SageMath/local/lib/python/fractions.pyc in __new__(cls, numerator, denominator) 152 isinstance(denominator, Rational)): 153 numerator, denominator = ( --> 154 numerator.numerator * denominator.denominator, 155 denominator.numerator * numerator.denominator 156 ) TypeError: unsupported operand type(s) for *: 'builtin_function_or_method' and 'builtin_function_or_method'
Change History (5)
comment:1 Changed 5 years ago by
comment:2 Changed 21 months ago by
- Cc jdemeyer mcbell slelievre added
- Keywords rational numerator denominator added
- Milestone changed from sage-7.3 to sage-duplicate/invalid/wontfix
- Reviewers set to Samuel Lelièvre
- Status changed from new to needs_review
Proposing to close as a duplicate of #28234 where a solution is being proposed.
comment:3 Changed 21 months ago by
To the OP: why do you care about Python fractions? It's a serious question since Guido van Rossum seems quite negative about them (see https://discuss.python.org/t/pep-3141-ratio-instead-of-numerator-denominator/2037/27?u=jdemeyer)
comment:4 follow-up: ↓ 5 Changed 21 months ago by
I filed the bug mainly because it seems like a bad idea to claim to implement interfaces that are not actually implemented. I probably should have been more clear about this, but I would consider just not registering the instances to be a solution to the problem.
comment:5 in reply to: ↑ 4 Changed 21 months ago by
- Resolution set to duplicate
- Status changed from needs_review to closed
OK, thanks for that info, it's useful to know! I'm closing this now as duplicate of #28234.
Honestly, I don't think this is easy to fix. Sage has a history of using methods for these and related things. So either we drop internal consistency within Sage or we drop compatibility with the numbers ABC.