search

pachterlab / voyager

From geospatial to spatial -omics
https://pachterlab.github.io/voyager/
Artistic License 2.0
70 stars 8 forks source link

Forthcoming spdep release seems to break Voyager #3

Closed rsbivand closed 1 year ago

rsbivand commented 1 year ago

00check.log testthat.Rout.zip

Please test your package with forthcoming spdep HEAD, main branch - I expect to release shortly, and BC packages are not picked up in regular reverse dependency checks (which are for CRAN, not BC). I see errors, but have no idea what your testthat usage expects. All the local conditional permutation functions are under active development and cannot be expected to have unchanging output. Please file isiues on spdep if need be. Do read https://github.com/r-spatial/spdep/issues/124 for details on some changes. In test-univariate.R:215, the extra output column comes from 1). In test-univariate.R:225, it is probably dropping the return_internals= argument. I am sceptical about backward-facing unit tests (nothing upstream can ever change) anyway.

Also see https://r-spatial.org/book/15-Measures.html#measures-and-process-mis-specification with regard to your docs splash on Tobler.

rsbivand commented 1 year ago

I am reverting the dropping of return_internals= in localG(), so only one problem remains in your unit tests.

lambdamoses commented 1 year ago

I have updated the tests in a new branch. I'll merge to main and push to Bioconductor git when you're finalizing the changes. Thanks for letting me know. The existence of the test for the output of localG() is to test how my package manages the local results when the output for each feature is a vector rather than a matrix. I pulled the newest version of spdep, and found that the return_internals argument of localG() is unused and the results always have the internals. So I removed that test, as it's no longer relevant.

As for Tobler's Law, I did clarify a bit in one of the vignettes about the misspecification problem and what causes spatial autocorrelation, though the clarification isn't in the most prominent place. I can elaborate more on that in the vignette with a formal definition of spatial autocorrelation and what may cause observed spatial autocorrelation in the histological space.

rsbivand commented 1 year ago

Thank you! I will try to remember to run reverse dependency tests on BioConductor packages when preparing releases, as unwittingly causing breakages isn't positive.