Implement Py3.9 LCM and GCD #1916
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
palaviv
requested changes
May 9, 2020
There was a problem hiding this comment.
Very nice work. I have left a small suggestion that should remove a lot of code.
Would you mind adding a comment on tests that are related to 3.9 features? We currently aim for 3.8 and I would prefer people to have that data before trying to fix that. Another options is to get the tests of 3.8 instead of 3.9.
vm/src/stdlib/math.rs
Outdated
| @@ -272,9 +272,55 @@ fn math_ldexp( | |||
| Ok(value * (2_f64).powf(objint::try_float(i.as_bigint(), vm)?)) | |||
| } | |||
|
|
|||
| fn math_gcd(a: PyIntRef, b: PyIntRef) -> BigInt { | |||
| fn math_perf_arb_len_int_op<F>( | |||
| args: PyFuncArgs, | |||
There was a problem hiding this comment.
You can use here Args<PyIntRef>. Look at PySetInner::update for example. This will remove some of the argument parsing code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the bpo-39648 and bpo-39479, which are planned for CPython 3.9.
This change extends the math module to implement a lcm (least common multiple) function analog to the gcd. Further both functions are extended to take an arbitrary number of int arguments.
The kind of args is in accordance to the 3.9.a6 cpython implementation, although this style of parameters is quite uncommon in the math module; in similar cases iterables are used as arguments. Maybe this changes before release.
Further, I imported the test_math.py module from CPython and skipped all failing tests (unluckily there are plenty of them)
Whats missing: cpython also accepts object arguments having index implemented, this seems so far not be finalized in the reference. Thus I defer the implementation.
replace #1907