JuliaStats / NullableArrays.jl

DEPRECATED Prototype of the new JuliaStats NullableArrays package
Other
35 stars 21 forks source link

short circuit anynull if eltype !<: Nullable #188

Closed cjprybol closed 7 years ago

cjprybol commented 7 years ago

example

using NullableArrays
using BenchmarkTools
X = rand(Int(1e6));
Y = rand(Int(1e4), Int(1e4));
@benchmark anynull(X)
@benchmark anynull(Y)

before

julia> @benchmark anynull(X)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     270.964 μs (0.00% GC)
  median time:      396.844 μs (0.00% GC)
  mean time:        373.259 μs (0.00% GC)
  maximum time:     917.353 μs (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

julia> @benchmark anynull(Y)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     27.749 ms (0.00% GC)
  median time:      29.160 ms (0.00% GC)
  mean time:        29.763 ms (0.00% GC)
  maximum time:     36.749 ms (0.00% GC)
  --------------
  samples:          168
  evals/sample:     1
  time tolerance:   5.00%
  memory tolerance: 1.00%

after

julia> @benchmark anynull(X)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     18.482 ns (0.00% GC)
  median time:      19.022 ns (0.00% GC)
  mean time:        20.409 ns (0.00% GC)
  maximum time:     87.697 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     997
  time tolerance:   5.00%
  memory tolerance: 1.00%

julia> @benchmark anynull(Y)
BenchmarkTools.Trial:
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     17.400 ns (0.00% GC)
  median time:      17.926 ns (0.00% GC)
  mean time:        19.375 ns (0.00% GC)
  maximum time:     82.290 ns (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     998
  time tolerance:   5.00%
  memory tolerance: 1.00%
codecov-io commented 7 years ago

Codecov Report

Merging #188 into master will increase coverage by 0.26%. The diff coverage is 90%.

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
+ Coverage   64.56%   64.82%   +0.26%     
==========================================
  Files          13       13              
  Lines         697      705       +8     
==========================================
+ Hits          450      457       +7     
- Misses        247      248       +1
Impacted Files Coverage Δ
src/primitives.jl 98.96% <90%> (-1.04%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b584c81...13263b9. Read the comment docs.

cjprybol commented 7 years ago

I'm not sure what the coverage drop is for in regards to coveralls, but I could open another PR to try and extend test coverage?

ararslan commented 7 years ago

Weird that Codecov went up negligibly and Coveralls fell significantly... not sure how to interpret that. I guess I'm inclined to call it a fluke, but I'll leave that up to @nalimilan.

cjprybol commented 7 years ago

All tests but coveralls are happy again. If @nalimilan is ok with these changes when the sun comes up on his side of the world, then we can merge and I'll followup with another PR to make the same changes for dropnull & dropnull! and add appropriate tests. Thanks for the help and reviews to both of you!