-
-
Notifications
You must be signed in to change notification settings - Fork 491
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
Allow construct RIF element from question-style string #38998
base: develop
Are you sure you want to change the base?
Allow construct RIF element from question-style string #38998
Conversation
Documentation preview for this PR (built with commit 805ecee; changes) is ready! 🎉 |
fc0b60b
to
14e1827
Compare
e8848d5
to
c9e5c99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; does what it says and is well tested.
We can worry about the integer case later on I think. |
Thanks! |
As in the title. It allows you to say e.g. `RIF("1.23?2e-5")`. Partially handles sagemath#36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.) (I don't see any disadvantage of allowing this, it's backwards compatible) Issue: currently ``` sage: RIF("10", base=37) 37 sage: ZZ("10", base=37) [error] ``` should this inconsistency be fixed? If so how? [Edit: actually the rule of conversion should probably follow [string to mpfr conversion rule](https://www.mpfr.org/mpfr-current/mpfr.html#index- mpfr_005fstrtofr) or [string to mpz conversion rule](https://gmplib.org/manual/Assigning-Integers) ] ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (there's no documentation change, but should we explicitly mention the feature? I think the feature to construct from `[a..b]` isn't explicitly mentioned either…?) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> sagemath#39001 URL: sagemath#38998 Reported by: user202729 Reviewer(s): Travis Scrimshaw
As in the title. It allows you to say e.g. `RIF("1.23?2e-5")`. Partially handles sagemath#36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.) (I don't see any disadvantage of allowing this, it's backwards compatible) Issue: currently ``` sage: RIF("10", base=37) 37 sage: ZZ("10", base=37) [error] ``` should this inconsistency be fixed? If so how? [Edit: actually the rule of conversion should probably follow [string to mpfr conversion rule](https://www.mpfr.org/mpfr-current/mpfr.html#index- mpfr_005fstrtofr) or [string to mpz conversion rule](https://gmplib.org/manual/Assigning-Integers) ] ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (there's no documentation change, but should we explicitly mention the feature? I think the feature to construct from `[a..b]` isn't explicitly mentioned either…?) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> sagemath#39001 URL: sagemath#38998 Reported by: user202729 Reviewer(s): Travis Scrimshaw
On 32-bit:
|
Most likely the binary exponent in 32-bit is limited to 2^30, and 10^10^9 > 2^2^30. Fix should be simple. (I don't have a 32-bit environment to test on, however experimentally on 64-bit environment |
As in the title. It allows you to say e.g. `RIF("1.23?2e-5")`. Partially handles sagemath#36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.) (I don't see any disadvantage of allowing this, it's backwards compatible) Issue: currently ``` sage: RIF("10", base=37) 37 sage: ZZ("10", base=37) [error] ``` should this inconsistency be fixed? If so how? [Edit: actually the rule of conversion should probably follow [string to mpfr conversion rule](https://www.mpfr.org/mpfr-current/mpfr.html#index- mpfr_005fstrtofr) or [string to mpz conversion rule](https://gmplib.org/manual/Assigning-Integers) ] ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (there's no documentation change, but should we explicitly mention the feature? I think the feature to construct from `[a..b]` isn't explicitly mentioned either…?) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> sagemath#39001 URL: sagemath#38998 Reported by: user202729 Reviewer(s): Travis Scrimshaw
As in the title. It allows you to say e.g. `RIF("1.23?2e-5")`. Partially handles sagemath#36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.) (I don't see any disadvantage of allowing this, it's backwards compatible) Issue: currently ``` sage: RIF("10", base=37) 37 sage: ZZ("10", base=37) [error] ``` should this inconsistency be fixed? If so how? [Edit: actually the rule of conversion should probably follow [string to mpfr conversion rule](https://www.mpfr.org/mpfr-current/mpfr.html#index- mpfr_005fstrtofr) or [string to mpz conversion rule](https://gmplib.org/manual/Assigning-Integers) ] ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (there's no documentation change, but should we explicitly mention the feature? I think the feature to construct from `[a..b]` isn't explicitly mentioned either…?) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> sagemath#39001 URL: sagemath#38998 Reported by: user202729 Reviewer(s): Travis Scrimshaw
As in the title. It allows you to say e.g. `RIF("1.23?2e-5")`. Partially handles sagemath#36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.) (I don't see any disadvantage of allowing this, it's backwards compatible) Issue: currently ``` sage: RIF("10", base=37) 37 sage: ZZ("10", base=37) [error] ``` should this inconsistency be fixed? If so how? [Edit: actually the rule of conversion should probably follow [string to mpfr conversion rule](https://www.mpfr.org/mpfr-current/mpfr.html#index- mpfr_005fstrtofr) or [string to mpz conversion rule](https://gmplib.org/manual/Assigning-Integers) ] ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. (there's no documentation change, but should we explicitly mention the feature? I think the feature to construct from `[a..b]` isn't explicitly mentioned either…?) ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> sagemath#39001 URL: sagemath#38998 Reported by: user202729 Reviewer(s): Travis Scrimshaw
As in the title. It allows you to say e.g.
RIF("1.23?2e-5")
.Partially handles #36797. (only for real case. Complex case is not handled yet, but in principle it should not be too difficult.)
(I don't see any disadvantage of allowing this, it's backwards compatible)
Issue: currently
should this inconsistency be fixed? If so how?
[Edit: actually the rule of conversion should probably follow string to mpfr conversion rule or string to mpz conversion rule ]
📝 Checklist
[a..b]
isn't explicitly mentioned either…?)⌛ Dependencies
#39001