koheiw / proxyC

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

Avoid deprecated Matrix coercions #36

Closed kbenoit closed 2 years ago

kbenoit commented 2 years ago

Further changes to avoid all warnings in calling Matrix deprecations.

I didn't add this to tests/testthat.R:

# return an error for all Matrix deprecations
options(Matrix.warnDeprecatedCoerce = 2)

since I will leave that choice up to you.

koheiw commented 2 years ago

Did you see warnings? I thought I switched to new functions in https://github.com/koheiw/proxyC/commit/4f23e2cbf6c1a0407190b56b73a93d8fa4bf8d86.

kbenoit commented 2 years ago

Yes there were still warnings with Matrix 1.5. (in dev only still). Turn on options(Matrix.warnDeprecatedCoerce = 2) and you will see them as errors, with traceback.

kbenoit commented 2 years ago

Here's the issue I am having. I have fixed quanteda.textstats for all existing and forthcoming coercion method deprecations - see https://github.com/quanteda/quanteda.textstats/pull/54 - but it still errors when calling proxyC because the current CRAN version (and your master branch) still call some deprecated coercions.

library("quanteda")
#> Package version: 3.2.3
#> Unicode version: 14.0
#> ICU version: 70.1
#> Parallel computing: 10 of 10 threads used.
#> See https://quanteda.io for tutorials and examples.
library("quanteda.textstats")

options(Matrix.warnDeprecatedCoerce = 2)

# from quanteda.textstats test-textstat_proxy.R:3-7
test_mt <- tokens(corpus_subset(data_corpus_inaugural, Year > 1980)) %>%
    tokens_remove(stopwords("en")) %>%
    tokens_wordstem("en") %>%
    dfm() %>%
    dfm_trim(min_termfreq = 5)

proxyC::simil(test_mt, NULL, 2, method = "jaccard")
#> Error: as(<dgCMatrix>, "lgCMatrix") is deprecated since Matrix 1.4-2; do as(., "lMatrix") instead

Created on 2022-09-02 with reprex v2.0.2

This is why the devel version of quanteda.textstats is throwing an error. I can tone down the Matrix warning level but it would be even better to change proxyC.

codecov[bot] commented 2 years ago

Codecov Report

Merging #36 (82c06c9) into master (a9002b5) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   99.49%   99.49%           
=======================================
  Files           4        4           
  Lines         395      395           
=======================================
  Hits          393      393           
  Misses          2        2           
Impacted Files Coverage Δ
R/proxy.R 99.07% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

koheiw commented 2 years ago

Thanks. Do you want me to submit the updated proxyC to CRAN now?

kbenoit commented 2 years ago

That would be great!

Kenneth Benoit Professor of Computational Social Sciences | Department of Methodology The London School of Economics and Political Science Houghton Street, London WC2A 2AE @.*** lse.ac.uk


From: Kohei Watanabe @.> Sent: Saturday, September 3, 2022 9:52:03 AM To: koheiw/proxyC @.> Cc: Benoit,KR @.>; Author @.> Subject: Re: [koheiw/proxyC] Avoid deprecated Matrix coercions (PR #36)

Thanks. Do you want me to submit the updated proxyC to CRAN now?

— Reply to this email directly, view it on GitHubhttps://github.com/koheiw/proxyC/pull/36#issuecomment-1235985607, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAQUYZRRFKCSBKQREWZHFZDV4KHKHANCNFSM6AAAAAAQCBCR4Y. You are receiving this because you authored the thread.Message ID: @.***>