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

Initiate hello-clusters #805

Merged

Conversation

sjspielman
Copy link
Member

Closes #793

This PR establishes the hello-clusters module:

  • I didn't include the analysis template directories (plots, scratch, & results) since I don't think they are necessary for this module, which I anticipate will just have some example notebooks at the top level. Do you foresee something I don't?
  • I added an initial description to the README, including a brief overview of the functions this module will present and how to install rOpenScPCA. As notebooks get added, the README will be expanded accordingly.
    • I put in an instruction for force=TRUE, but I don't think I like that I did that? I imagine when there are relevant-enough changes we'll bump the package version itself and renv::update() maybe should be used instead?
  • I added an initial (mostly empty) notebook which will get filled in as part of Add notebook to demonstrate clustering with rOpenScPCA #796. I added this to be able to load libraries that allowed me to build up the renv.lock file. I also added an, essentially empty for now, script to run the module which will likely just be used for testing
  • The Dockerfile is ready to go as well, but I didn't activate the build docker workflow yet. But, if we want to actually just get the run module workflow up and running with a Docker environment from the get-go, maybe now is actually the time so the next PRs can use it!

@sjspielman sjspielman requested review from jashapiro and removed request for jaclyn-taroni October 9, 2024 17:33
Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good for the start of the repo. One typo fix I saw, and the comments below (tl;dr: do add back the standard directories).

I had some interesting trouble installing at first with your directions and a slightly nonstandard renv setup, but I think the fix for that can go elsewhere.

  • I didn't include the analysis template directories (plots, scratch, & results) since I don't think they are necessary for this module, which I anticipate will just have some example notebooks at the top level. Do you foresee something I don't?

I would include these, as they don't hurt at all. You can also have the notebooks save some results (multiple clustering results as a table) and plots. I think it is good to demonstrate this even in a simple module like this.

cancel-in-progress: true

on:
# pull_request:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any reason not to activate this if it is ready to go.

analyses/hello-clusters/01_perform-evaluate-clustering.Rmd Outdated Show resolved Hide resolved
analyses/hello-clusters/README.md Outdated Show resolved Hide resolved
analyses/hello-clusters/README.md Outdated Show resolved Hide resolved
@sjspielman sjspielman merged commit ad0a80e into AlexsLemonade:main Oct 10, 2024
4 checks passed
@sjspielman sjspielman deleted the sjspielman/793-well-hello-clusters branch October 10, 2024 18:48
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.

Establish module to demonstate clustering with rOpenScPCA
2 participants