vegandevs / vegan

R package for community ecologists: popular ordination methods, ecological null models & diversity analysis
https://vegandevs.github.io/vegan/
GNU General Public License v2.0
448 stars 97 forks source link

Adding `pca` and `ca` (wish of #655) breaks two packages in reverse dependencies tests #664

Open jarioksa opened 3 months ago

jarioksa commented 3 months ago

The reason for breaking two packages with #655 seem to be the same: functions ca and pca are defined in other packages and when they are also defined in vegan, our functions will be used with disastrous results.

In package PLSDAbatch:

* checking whether package ‘PLSDAbatch’ can be installed ... WARNING
Found the following significant warnings:
  Warning: replacing previous import ‘mixOmics::pca’ by ‘vegan::pca’ when loading ‘PLSDAbatch’

and this precipitates as an ERROR when vegan::pca is used instead of mixOmics::pca. NAMESPACE has explicit import of mixOmics::pca, but as the last command a sweeping vegan import where the replacement happens:

importFrom(mixOmics,pca)
...
import(vegan)

Package easyCODA has similar story:

* checking whether package ‘easyCODA’ can be installed ... WARNING
Found the following significant warnings:
  Warning: replacing previous import ‘ca::ca’ by ‘vegan::ca’ when loading ‘easyCODA’

With an error when easyCODA::CA uses vegan::ca instead of ca::ca. Here the usage in the NAMESPACE is even more sweeping:

import(ca, vegan, ellipse)
jarioksa commented 3 months ago

Using names like pca and ca is risky: many other people use them, and then we have name clash. A quick survey in packages that depend on vegan found that the following packages export pca: ProcMod, SYNCSA, mixOmics, and ca is exported from ca.

gavinsimpson commented 3 months ago

The issue with the two packages that get broken is clearly a "their problem" not ours - I'll contact the maintainers of both packages to advise more defensive (selective) imports or whatever is needed to fix them

gavinsimpson commented 3 months ago

As regards risky name clashes, this is unavoidable given the maturity of the R platform and its package ecosystem. We need to balance what's best for vegan and our users with potential problems with other packages. Users already deal with these things with dplyr for example trampling on filter() and select(), but dplyr does this for good, justifiable reasons, and I see that vegan::ca() and vegan::pca() are justifiable and good additions to vegan even if they cause some short term grief with broken packages.

Beyond the broken packages, the only people that will get hit with these new functions in vegan are those people that load all of the packages in their scripts without giving this bad practice any thought.

gavinsimpson commented 3 months ago

I have emailed Michael Greenacre (easyCODA) and opened an Issue on PLSDAbatch EvaYiwenWang/PLSDAbatch#4 to let them know about the problem.

gavinsimpson commented 3 months ago

Looking at PLSDAbatch, I don't see any calls to vegan functions in the package code. One example uses vegan::varpart(), so I will prepare a PR that moves vegan to Suggests, deletes the entry in NAMESPACE and then makes the example code run only if vegan is available.

gavinsimpson commented 3 months ago

Micheal has just replied acknowledging my report of the problem and stating that he'll prepare updates to easyCODA.

gavinsimpson commented 3 months ago

I have prepared a PR EvaYiwenWang/PLSDAbatch#5 that fixes the problems with PLSDAbatch

gavinsimpson commented 1 month ago

I have sent patches to Michael Greenacre for easyCODA's DESCRIPTION and NAMESPACE which fix this issue in that package.