Sorry if this is off topic, but why suggest turning
x === missing to
ismissing(x)? Especially when it seems to be fine with
x === nothing?
Sorry if this is off topic, but why suggest turning
@tlienart Thanks! I’ve added an issue for your first suggestion and am going to improve on that. For your second suggestion, I think that’s not possible yet. The site shows too many false positives. So far, I did find some interesting problems in code and created PRs manually:
@Mason that’s not off topic. Are you sure that
x === missing is the same as
x === nothing? I’m not an expert, so it could definitely be true. Would you mind elaborating a bit more?
nothing are defined as
Nothing() respectively, and
isnothing are both defined as
ismissing(::Any) = false ismissing(::Missing) = true
isnothing(::Any) = false isnothing(::Nothing) = true
respectively, so as far as I can tell, they’re exactly analogous.
I believe it’s usually recommended to use
x === nothing and
x === missing when possible because the compiler is able to optimize it in more circumstances. From what I understand,
isnothing are mostly useful for higher order functions, e.g.
I do expect that ismissing(x) is easier to read for new Julia users, and performance is not really the issue for most practical problems („premature optimization is the root of all evil“), but you might very well be right that its not an useful pattern to check for. I‘m gonna look into it a bit more.
Maybe these are relevant:
I’ve opened a topic at Ismissing(x) versus x === missing.
That’s pretty dope and I’m happy that you analysed two of my repos
How long does it take to produces the results? Is it out of reach to do this for all registered packages?
Haha, you’re welcome
@joa-quim In response to https://discourse.julialang.org/t/ismissing-x-versus-x-missing/52171/3: That’s why I’m working ShowLint with the possibility to disable lint patterns in bulk. Instead of adding a lint configuration to each project/repository, I could just disable the check for this pattern and similar checks on all “high-performance” repositories. Basically, the idea is that all the configuration complexity is moved away from the user and is managed at one place. Also, the idea is that its easier for the Julia community to help busy package creators out. For example, some packages are created one or more years ago and the creator has moved on to other things. With something like ShowLint, the community can more easily assist these creators in keeping their packages working and up-to-date.
Oh and maybe only lint the
src folder as something like
a == a can be helpful in testing to check if the function for
== is correctly implemented.
You’re quick. It took me until this morning to realize that https://github.com/rikhuijzer/ShowLint/issues/9
Comparison with Boolean constants are only redundant if you can prove that the value itself is a
b == trues(n) is not equivalent to
Haha I didn’t look at the issues though I saw that you changed some
a == a in the latest run. Have you tackled the false positives in JuMP.jl for example? There you have
@constraint(m, x-1 == 1) and form it into
@GunnarFarneback Yes, I’m still tweaking the rules and am note sure what to do with it. I do think that https://github.com/JuliaData/YAML.jl/pull/103 shows that there are situations where that check can be useful but, indeed, its a part of Julia since
x == true will give
x is a missing.
@Wikunia I got some tips from Rijnard van Tonder (Comby’s creator) and expect to have that fixed later today
This is great. I’d love to see more linting in Julia. A few points:
a == aappears to also trigger on
2 * a == a * 2and even
x/a == a, which should not be intended.
x -> f(x)triggers on
x -> 2gx(x), suggesting
2gx, which is not corret
Thanks for both. Good catch on
x/a == a. Should be fixed somewhere today.
Looks like a neat tool!
My few points:
- The replacement for
findall(x -> x==false, [true, false])should probably be
findall(!, [true, false])rather than broadcasting.
- But replacing
cond == falsewith
condonly seems safe if you are sure that
cond::Bool. I don’t see examples where this is clearly violated, but I do have functions which expect either
findall(!, [true, false, nothing])obviously won’t work.
Thanks for your suggestions!
I’m afraid that I don’t understand what would be the difference between
findall(!, A) and
findall(.!A), and why the former would be preferred. Could you explain the difference?
So, this is now the second pattern which doesn’t handle missings well. Thanks for pointing that out
My thinking was that this would save allocating an array here. Although for
findall in particular, it might not actually be faster?
julia> @btime findfirst(x->!x, r) setup=(r=rand(Bool, 10^6)); # same as findfirst(!, r) 2.283 ns (0 allocations: 0 bytes) julia> @btime findfirst(.!r) setup=(r=rand(Bool, 10^6)); 137.891 μs (4 allocations: 126.42 KiB) julia> @btime count(x->!x, r) setup=(r=rand(Bool, 10^6)); 80.810 μs (0 allocations: 0 bytes) julia> @btime count(.!r) setup=(r=rand(Bool, 10^6)); 142.010 μs (4 allocations: 126.42 KiB) julia> @btime findall(x->!x, r) setup=(r=rand(Bool, 10^6)); 671.744 μs (8 allocations: 3.92 MiB) julia> @btime findall(.!r) setup=(r=rand(Bool, 10^6)); 668.053 μs (6 allocations: 3.93 MiB)
Edit – that’s because the second-last line explicitly calls the last here,
findall(testf::Function, A::AbstractArray) = findall(testf.(A)), so there can’t be a difference. Sorry about the noise!
This is a good idea and I hope it can gain some more traction / incorporation into IDEs. Traceur.jl has a similar design space and is perhaps worth looking into.