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

fastTopics wrapper #151

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

fastTopics wrapper #151

wants to merge 55 commits into from

Conversation

pcarbo
Copy link

@pcarbo pcarbo commented Mar 20, 2023

Dear Seurat Developers,

I'm submitting for your consideration a simple Seurat wrapper for the fastTopics package.

The most recent methods are described in a bioRxiv preprint.

I tried to follow the "contribution guide", but please let me know if you would like me to make any changes to the interface or implementation.

Thank you,
Peter

… fixed the handling of the features (in particular, we avoid using VariableFeatures).
@pcarbo
Copy link
Author

pcarbo commented Nov 9, 2023

Hi Seurat Developers,

The paper describing these methods is now published in Genome Biology.

Please let me know if I should update the fastTopics wrapper code to address any new requirements or compatibility issues with the latest version of Seurat.

Thank you!
Peter

@pcarbo
Copy link
Author

pcarbo commented Jan 11, 2024

Hi, now that Seurat 5 is released on CRAN, would now be a good time to revisit this pull request?

Should I make sure that this wrapper is compatible with Seurat 5, and update it as needed?

@samuel-marsh
Copy link

Just adding a +1 in support of Seurat5 support and +1 to Seurat Team in terms of merging into SeuratWrappers.

@pcarbo in terms of updating to Seurat 5 it's pretty straightforward (from my brief testing). Only changes I had to make were:

  • LayerData instead of GetAssayData. (Line 447)
  • Use rlang::check_installed instead of CheckPackage (3 instances).
  • Depending on how you want users to be able to use it either add support for individual layers or add a check to require users to run JoinLayers before running fastTopics.

Best,
Sam

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

Successfully merging this pull request may close these issues.

2 participants