Conversation
numpy/core/src/umath/loops.h.src
Outdated
There was a problem hiding this comment.
This commit's diff is just combining U@TYPE@_absolute and @TYPE@_absolute into @S@@TYPE@_absolute, and the same for _sign, _divide, and _remainder. This is already the pattern we use for other functions in this file
|
Not the most efficient of algorithms - CPython 3.5 uses Lehmer's algorithm in Also, the builtin |
160c2a5 to
264c1c7
Compare
|
Ok, tests are added
|
f881013 to
24774f6
Compare
|
All green, documented, and release note'd |
|
☔ The latest upstream changes (presumably #8795) made this pull request unmergeable. Please resolve the merge conflicts. |
numpy/core/src/umath/loops.c.src
Outdated
There was a problem hiding this comment.
The standard codes for these are B, H, I, L and Q. Shouldn't you use those instead?
There was a problem hiding this comment.
Possibly. Although I didn't see much point implementing B, H, and I separately, because arithmetic always promotes to int in C anyway, doesn't it?
Can you link me to a file that uses these standard codes? Oh, these are the struct.pack codes, right? I was trying to write this as though it were a free C function, like labs and llabs are to abs. I guess in hindsight they use a prefix though...
There was a problem hiding this comment.
A better example of what I was imitating might be strto{l,ll,ul,ull} from the C standard library.
There was a problem hiding this comment.
...these are the struct.pack codes, right?
They are also documented in numpy; see https://docs.scipy.org/doc/numpy/reference/arrays.scalars.html#built-in-scalar-types and numpy.typecodes['Integer'], numpy.typecodes['UnsignedInteger'], etc.
|
☔ The latest upstream changes (presumably #8847) made this pull request unmergeable. Please resolve the merge conflicts. |
d282345 to
40b4bc8
Compare
|
Would be quite nice to have this! To me, it looks OK, but maybe e.g. @juliantaylor can have a look? |
adca9d6 to
36d6cc7
Compare
There was a problem hiding this comment.
Got this wrong until now. The identity of gcd is 0 since by definition, gcd(a, 0) == a.
The identity for lcm is some kind of epsilon. For the integers, that epsilon is simply 1 (lcm(a, 1) = a). However, for the rationals, (ie decimal.Decimal), this is incorrect (lcm(0.2, 1) != 0.2), nor is such an epsilon really representable anyway.
|
☔ The latest upstream changes (presumably #9063) made this pull request unmergeable. Please resolve the merge conflicts. |
36d6cc7 to
7278d17
Compare
|
Rebased for 1.14 |
7278d17 to
d1513be
Compare
|
I looked again at this, and it all looks fine, except there was the question about which character to use. Also, I'd explicitly check that calling the functions with a float gets you an error, and maybe that it works for a python super-long integer. |
Now works for builtin `long`, `Decimal`, and `Fraction` types
|
@mhvk: Tests and release notes fixed. I don't think there's a strong argument to be made about the suffices - since they're a private implementation detail, I'd prefer to leave them |
|
OK, that looks great. Merging! |
|
Thanks! Glad I won't have to rebase for 1.16 too! |
Fixes #8772, implementing Least Common Multiple and Greatest Common Divisor as
ufuncsSince these are now both in the C++17 stdlib, it seems pretty reasonable that they should be part of numpy as well. Specification matches the C++17 one, where the result is always positive.
Defined for:
intsobjectlongs defer tomath.gcd(if available)Decimal,Fraction, ...) defers tofraction.gcdnp.core.internal._gcd, but enforcing consistency withmath.gcd(returning a positive value)Not defined for:
bool- doesn't seem useful, since it just decays tological_ornp.inexact- not well definedFirst commit is some minor cleanup to make it easier to see where to add new integer ufuncs. Githubs rendering of this diff is not very clear