Fix #9389 Uint handling for searchsorted* methods
#9936
Conversation
| @test searchsorted([1:10], 10, by=(x -> x >= 5)) == 5:10 | ||
| @test searchsorted([1:5, 1:5, 1:5], 1, 6, 10, Base.Order.Forward) == 6:6 | ||
| @test searchsorted(ones(15), 1, 6, 10, Base.Order.Forward) == 6:10 | ||
| for R in [Int, Uint8, Float64], T in [Int, Uint8, Float64] |
There was a problem hiding this comment.
Seems like the list of types could be much longer? Why don't test BigInt, BigFloat, UInt64, UInt128....?
|
That's true, adding them now. |
|
Now the tests seem to fail for searchsortedfirst(1:3, UInt128(2))
ERROR: error compiling searchsortedfirst: cannot convert type to a julia typewith the same thing when using Running |
|
Cc: @kmsquire |
|
Seems to be a compiler bug. See #9947. |
|
@darwindarak Seems like you can just add the types that work, and leave a comment in #9947 about the 128 bit failures and how the failing tests should be re enabled when that bug is fixed. |
|
@darwindarak Github doesn't push out notifications when you push commits, so you should usually add a comments when you push. Can someone review the correctness of this? |
|
I just found a problem with my implementation that I'm working on fixing right now. I forgot that the promotion rules for |
searchsortedfirst method to find Uint in rangessearchsorted* methods
|
I think I fixed it. |
|
It should be converted to an Int. BTW, the fix for the bug above has been committed, so Int128 and UInt128 should work and can be uncommented. |
|
Since it's such a trivial change, I think it's fine to just leave it here. This looks ready to go! Can you please squash the commits? |
|
Squashed! |
Add more types to `searchsorted` tests Testing with *Int128 types is triggering an compiler error, so they are commented out for now. We can add them back to the once #9947 is fixed Convert Unsigned to Signed before subtraction Fix Uint handling for `searchsortedlast` Re-enable `*Int128` tests for searchsorted` Previously blocked by #9947 Make `Float16`, `*Int128` promote to `Float32`
|
Rebased again to include changes from #10003. |
Fix #9389 Uint handling for `searchsorted*` methods
|
@JuliaBackports. Depends on #9952. |
Add more types to `searchsorted` tests Testing with *Int128 types is triggering an compiler error, so they are commented out for now. We can add them back to the once #9947 is fixed Convert Unsigned to Signed before subtraction Fix Uint handling for `searchsortedlast` Re-enable `*Int128` tests for searchsorted` Previously blocked by #9947 Make `Float16`, `*Int128` promote to `Float32` (cherry picked from commit c402273, syntax adjusted for 0.3) ref PR #9936
|
backported in 707f94f |
searchsortedfirst(a::Range{Integer}, x::Real)is not working as expected whentypeof(x) == UInt8. This is caused byfloor(Integer, -x), where the negation of the unsignedxdoes not give us what we need. This PR adds a method forsearchsortedfirstjust to handle unsignedx.I suppose you could just do
floor(Integer, -1x)orfloor(Integer, 0-x), but that feels wrong somehow.