gpuR icon indicating copy to clipboard operation
gpuR copied to clipboard

Add the sum method for vclVector objects

Open daydreamt opened this issue 7 years ago • 3 comments

Dear Mr Determan, thank you for your efforts with gpuR!

While using it I noticed there is no sum() method for vclVectors, so I went ahead and added one, based on the one for vclMatrix. Please let me know if I can do anything more, or differently, to get it merged.

daydreamt avatar Mar 03 '18 22:03 daydreamt

Codecov Report

Merging #115 into develop will decrease coverage by 0.15%. The diff coverage is 7.69%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #115      +/-   ##
===========================================
- Coverage    81.39%   81.23%   -0.16%     
===========================================
  Files           58       58              
  Lines        12399    12424      +25     
===========================================
+ Hits         10092    10093       +1     
- Misses        2307     2331      +24
Impacted Files Coverage Δ
R/wrappers-vclVector.R 44.44% <0%> (-14.1%) :arrow_down:
R/methods-vclVector.R 71.34% <100%> (+0.08%) :arrow_up:
src/vienna_stats.cpp 72.96% <8.33%> (-1.77%) :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 b6527e8...7edb41c. Read the comment docs.

codecov-io avatar Mar 03 '18 23:03 codecov-io

Coverage Status

Coverage decreased (-0.2%) to 81.238% when pulling 7edb41cd6d0cd1ee290ef847b3fe1229ef5b88f1 on daydreamt:develop into b6527e84a2df7a7ceadadd3da2adee96e4339baa on cdeterman:develop.

coveralls avatar Mar 03 '18 23:03 coveralls

Thanks for the contribution! If you could, I would appreciate you adding the unit tests as well for the new method. Given that sum is part of the Summary generic I would say add this to the tests/testthat/test_cpu_vclVector_math.R and tests/testthat/test_vclVector_math.R files.

cdeterman avatar Mar 12 '18 14:03 cdeterman