search

rformassspectrometry / Spectra

Low level infrastructure to handle MS spectra
https://rformassspectrometry.github.io/Spectra/
37 stars 25 forks source link

Remove all functionality related to Rle-based compression #85

Closed jorainer closed 4 years ago

jorainer commented 4 years ago

Remove all functionality for Rle-based functionality from MsBackendDataFrame (and in fact from the whole package) - as discussed in issue #83 .

jorainer commented 4 years ago

Sorry - still having some issues with the tests - should be fixed soon

jorainer commented 4 years ago

OK, now it's ready @sgibb if you want to cross-check.

codecov-io commented 4 years ago

Codecov Report

Merging #85 into master will decrease coverage by 0.18%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   90.33%   90.14%   -0.19%     
==========================================
  Files          16       16              
  Lines        1479     1451      -28     
==========================================
- Hits         1336     1308      -28     
  Misses        143      143
Impacted Files Coverage Δ
R/MsBackend.R 3.12% <ø> (ø) :arrow_up:
R/functions-util.R 100% <ø> (ø) :arrow_up:
R/MsBackendDataFrame.R 97.86% <100%> (-0.11%) :arrow_down:
R/MsBackendMzR.R 97.05% <100%> (-0.17%) :arrow_down:
R/MsBackendHdf5Peaks.R 90.28% <100%> (-0.06%) :arrow_down:
R/MsBackendMzR-functions.R 93.84% <100%> (ø) :arrow_up:
R/MsBackendDataFrame-functions.R 95.18% <100%> (-0.17%) :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 53a46f6...0ec6cdf. Read the comment docs.

jorainer commented 4 years ago

I agree that your proposed channge would remove some lines of code, but I think we can live with this if we want to allow value of either be length 1 or equal nrow of the data frame. I have a small preference to keep the if/else to avoid the R-inherit replication but am also fine if you insist on removing it.