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.

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

Invalid mpq.__float__? #498

Closed
skirpichev opened this issue Jul 22, 2024 · 5 comments · Fixed by #499
Closed

Invalid mpq.__float__? #498

skirpichev opened this issue Jul 22, 2024 · 5 comments · Fixed by #499

Comments

@skirpichev
Copy link
Contributor

Consider following example:

>>> from gmpy2 import mpq
>>> from fractions import Fraction
>>> float(Fraction(9, 5))
1.8
>>> float(mpq(9, 5))
1.7999999999999998

It seems, gmpy2 uses mpq_get_d(), which "Convert op to a double, truncating if necessary (i.e. rounding towards zero)". While Python does round to nearest.

Indeed:

>>> mpz(9)/mpz(5)
mpfr('1.8')
>>> ctx = gmpy2.get_context()
>>> ctx.round=gmpy2.RoundToZero
>>> mpz(9)/mpz(5)
mpfr('1.7999999999999998')

I'll provide a patch.

@skirpichev
Copy link
Contributor Author

@casevh, will you include this (57378b6) in the next point release?

@casevh
Copy link
Collaborator

casevh commented Aug 5, 2024 via email

@skirpichev
Copy link
Contributor Author

@casevh, do you think it's a good idea to keep the master branch for 2.2.x point releases? I believe it's better to use a dedicated branch (say, 2.2) and cherry-pick only some commits from the master branch.

@casevh
Copy link
Collaborator

casevh commented Aug 5, 2024 via email

@skirpichev
Copy link
Contributor Author

Is a fix for the fallthrough warnings needed for 2.2.2?

If you meant commentaries, I think it's ok: that's just a couple of commentaries:)

But if you about #502 - no. This PR looks unsuitable even for the master, unless OP provide a portable way to do deal with such warnings. I don't think we should require C23, thus probably this PR should be rejected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants