Skip to content

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

Merged
skirpichev merged 5 commits intompmath:masterfrom
skirpichev:rationals-from-backend
May 19, 2023
Merged

Drop private mpq class, use Rational's, provided by backend#691
skirpichev merged 5 commits intompmath:masterfrom
skirpichev:rationals-from-backend

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

oscarbenjamin commented May 18, 2023

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

oscarbenjamin commented May 19, 2023

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

skirpichev commented May 20, 2023

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 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