-
Notifications
You must be signed in to change notification settings - Fork 97
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
Comments
pca
and ca
(wish of #655) break two package in reverse dependencies testspca
and ca
(wish of #655) breaks two packages in reverse dependencies tests
Using names like |
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 |
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 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. |
I have emailed Michael Greenacre (easyCODA) and opened an Issue on PLSDAbatch EvaYiwenWang/PLSDAbatch#4 to let them know about the problem. |
Looking at PLSDAbatch, I don't see any calls to vegan functions in the package code. One example uses |
Micheal has just replied acknowledging my report of the problem and stating that he'll prepare updates to easyCODA. |
I have prepared a PR EvaYiwenWang/PLSDAbatch#5 that fixes the problems with PLSDAbatch |
I have sent patches to Michael Greenacre for easyCODA's |
PLSDAbatch is a Bioconductor package and they will have timed releases of the whole of Bionductor. When writing this, Bioconductor was at 3.20 released Oct 20, 2024. This release did not fix the issue. However, it seems to be fixed in their github repository. As of today, PLSDAbatch had some other issues (errors) in Bionconductor. These seemed to be unrelated to confusion with |
Contrary to my previous statement, the problem was solved in BioConductor 3.20 with PLSDAbatch 1.2.0. |
The reason for breaking two packages with #655 seem to be the same: functions
ca
andpca
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 ofmixOmics::pca
. NAMESPACE has explicit import ofmixOmics::pca
, but as the last command a sweeping vegan import where the replacement happens: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
usesvegan::ca
instead ofca::ca
. Here the usage in the NAMESPACE is even more sweeping:The text was updated successfully, but these errors were encountered: