8000 Drop private mpq class, use Rational's, provided by backend by skirpichev · Pull Request #691 · mpmath/mpmath · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Drop private mpq class, use Rational's, provided by backend #691

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 19, 2023
Merged

Drop private mpq class, use Rational's, provided by backend #691

merged 5 commits into from
May 19, 2023

Conversation

skirpichev
Copy link
Collaborator
@skirpichev skirpichev commented Apr 25, 2023 •
edited
Loading
  • Missing feature (for pure-python) is an additional caching mechanism. Not sure it worth.
  • Unfortunately, gmpy2 has no released version with support for mpq.as_numer_denom(), hence I use foo.numerator/denominator to unpack components of rationals... Edit: the numbers.Rational doesn't require this method, so we could not rely on it.

@skirpichev
Copy link
Collaborator Author
skirpichev commented Apr 26, 2023 •
edited
Loading

Hmm, can't reproduce this failure locally :(

Solved issue with low precision for Fraction**float ```python $ cat a.py import mpmath mpmath.mp.dps = 25 mpmath.mp.pretty = True r = mpmath.zeta(-10.5, (-8,3)) print(r) $ python a.py # ok (-0.01521913704446246609237979 + 29907.72510874248161608216j) $ MPMATH_NOGMPY=Y python a.py # wrong im? (-0.01521913704446246609237979 + 29907.72510874246440785656j) ```

@skirpichev skirpichev force-pushed the rationals-from-backend branch 3 times, most recently from 31893fc to ddd46c8 Compare April 29, 2023 06:09
@skirpichev skirpichev mentioned this pull request Apr 29, 2023
Closed
@skirpichev skirpichev force-pushed the rationals-from-backend branch 2 times, most recently from 5c78e94 to f519d25 Compare May 10, 2023 06:24
@skirpichev skirpichev mentioned this pull request May 11, 2023
Closed
@skirpichev skirpichev force-pushed the rationals-from-backend branch 2 times, most recently from 9dede89 to c88cac7 Compare May 12, 2023 06:27
@oscarbenjamin
Copy link

This looks good to me.

I don't know how significant the caching mechanism is but it does not look as if there are any places where MPQ is used in a tight loop.

@skirpichev
Copy link
Collaborator Author
skirpichev commented May 19, 2023 •
edited
Loading

@oscarbenjamin, I've some doubts about the ctx.mpq attribute. It's not documented, but apparently people discover it and use (e.g. #532). Maybe we should keep this and issue a deprecation warning before removing?
Edit: the mpq class was documented as "for internal use" (in its docstring), so I think it's safe to remove the attribute without a deprecation period.

@skirpichev skirpichev force-pushed the rationals-from-backend branch from e000e63 to 3fd10c6 Compare May 19, 2023 03:15
@skirpichev skirpichev force-pushed the rationals-from-backend branch from 3fd10c6 to 920d8ed Compare May 19, 2023 04:03
@skirpichev skirpichev merged commit 4477775 into mpmath:master May 19, 2023
@skirpichev skirpichev deleted the rationals-from-backend branch May 19, 2023 04:22
@oscarbenjamin
Copy link

Maybe we should keep this and issue a deprecation warning before removing?

Oh, I didn't notice that this removes the ctx.mpq attribute as well as ctx._mpq. I would say that as long as MPQ exists then it is harmless to just leave ctx.mpq as an alias to it. If there is no significant gain in removing it then I don't see any need to deprecated it.

@skirpichev
Copy link
Collaborator Author

to just leave ctx.mpq as an alias to it.

mpq is a special attribute for the MPContext() only.

If there is no significant gain in removing

Slightly better API?

@oscarbenjamin oscarbenjamin mentioned this pull request May 21, 2023
Open
@skirpichev skirpichev added this to the 62BA 1.4 milestone May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.
2 participants
@skirpichev @oscarbenjamin
0