Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jarioksa opened this issue Jun 23, 2024 · 7 comments

Comments

@jarioksa
Copy link
Contributor

jarioksa commented Jun 23, 2024

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 jarioksa changed the title Adding pca and ca (wish of #655) break two package in reverse dependencies tests Adding pca and ca (wish of #655) breaks two packages in reverse dependencies tests Jun 23, 2024
@jarioksa
Copy link
Contributor Author

jarioksa commented Jun 23, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@gavinsimpson
Copy link
Contributor

gavinsimpson commented Jun 23, 2024

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
Copy link
Contributor

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

@gavinsimpson
Copy link
Contributor

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

@jarioksa jarioksa mentioned this issue Jun 24, 2024
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants