search

koheiw / proxyC

R package for large-scale similarity/distance computation
GNU General Public License v3.0
29 stars 6 forks source link

Issue 28 #29

Closed koheiw closed 2 years ago

koheiw commented 2 years ago

Address #28

codecov[bot] commented 2 years ago

Codecov Report

Merging #29 (72efad1) into master (42da804) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #29   +/-   ##
=======================================
  Coverage   99.46%   99.46%           
=======================================
  Files           4        4           
  Lines         371      371           
=======================================
  Hits          369      369           
  Misses          2        2
Impacted Files Coverage Δ
R/proxy.R 99.06% <100.00%> (ø)

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 42da804...72efad1. Read the comment docs.

rcannood commented 2 years ago

Sorry for the delay!

This PR makes sense to me.

One thought. Would it make sense to use as(., "CsparseMatrix") instead if as(., "dgCMatrix")? They both result in a dgCMatrix (afaik), but using CsparseMatrix has worked better for me in some cases.

koheiw commented 2 years ago

Thank you for the review. You are right that symmetric matrices error with as(x, "dgCMatrix"), so changed to x <- as(as(x, "CsparseMatrix"), "dgCMatrix").