waldronlab / lefser

R implementation of the LEfSe method
https://waldronlab.io/lefser/
38 stars 6 forks source link

use of unexported function from EnrichmentBrowser #6

Closed lwaldron closed 3 years ago

lwaldron commented 3 years ago

Using unexported functions from other packages should usually be avoided, because there's no guarantee they will continue to be available or stay the same. What's the rationale for this use? If it is something simple, could you just copy this function to the package?

https://github.com/waldronlab/lefser/blob/9cfe8ea18145cbe047482dad6fe03daa43b7f245/R/lefserAnalysis.R#L21

lwaldron commented 3 years ago

Note that importing EnrichmentBrowser adds quite a few dependencies... definitely want to think about whether this is really necessary for lefser.

asyakhl commented 3 years ago

I replaced EnrichmentBrowser:::.extractInfoFromSE(se) with code of my own.

asyakhl commented 3 years ago

I also removed EnrichmentBrowser as one of imports from DESCRIPTION file, because actually we want EnrichmentBrowser to depend on lefser not the other way around.

lwaldron commented 3 years ago

Good. Note, if you put a note in your commit message like "closes #6" it will link to this issue and close it automatically. Otherwise, in your closing message for this issue you can paste the hash of the relevant commit (looks like eb8c94f?)

asyakhl commented 3 years ago

Yes, that is the one.