welch-lab / liger

R package for integrating and analyzing multiple single-cell datasets
GNU General Public License v3.0
389 stars 78 forks source link

inconsistencies with tsne/umap projection helper functions #162

Closed jfx319 closed 4 years ago

jfx319 commented 4 years ago

In runUMAP(), why are the coordinates stored under object@tsne_coords -- shouldn't this be named umap_coords? The runTSNE() overwrites this.... I would think you'd want to keep them separate as they are different projections and mean different styles for users.

In runTSNE(), the current liger code makes the user specify a FIt-SNE folder path, just to import the fast_tsne.R wrapper, which then has some hardcoding of the fast_tsne binary location anchored to that folder. Although not ideal and somewhat round about, the more pressing problem is that the liger.R currently comments out loading of this wrapper's function (implying the user needs to source() the wrapper manually (probably not what you would want):

jfx319 commented 4 years ago

ah.. I see that a utilities.R is in progress but not ready yet -- that should eventually address the fast_tsne part above

samuel-marsh commented 4 years ago

Looks like the ability to store both tsne and UMAP is also in progress but no changes have been made in while, see #94 I'd second the desire for the ability to store multiple dim reductions in the same object!

Thanks! Sam

skpalan commented 4 years ago

Hi Jeff and Sam, thanks for your comments. We fixed fast_tsne issue a long time ago, so it shouldn't be a problem anymore. For the dim reduction coords issue, it has been on our list for a while and it will be addressed in the future.