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

initial commit #766

Merged
merged 6 commits into from
Sep 20, 2024
Merged

Conversation

danhtruong
Copy link
Contributor

Purpose/implementation Section

Initial commit with the readme.md.

Please link to the GitHub issue that this pull request addresses.

If applicable, you can also link to the associated GitHub Discussion.
#701

What is the goal of this pull request?

Initialize the module and add the readme.md

Briefly describe the general approach you took to achieve this goal.

Created a fork and a feature branch.

If known, do you anticipate filing additional pull requests to complete this analysis module?

Yes. I will add the analysis script.

Provide directions for reviewers

Initial commit and confirming the workflow

Is there anything that you want to discuss further?

Author checklists

Check all those that apply.
Note that you may find it easier to check off these items after the pull request is actually filed.

Analysis module and review

Reproducibility checklist

  • Code in this pull request has been added to the GitHub Action workflow that runs this module.
  • The dependencies required to run the code in this pull request have been added to the analysis module Dockerfile.
  • If applicable, the dependencies required to run the code in this pull request have been added to the analysis module conda environment.yml file.
  • If applicable, R package dependencies required to run the code in this pull request have been added to the analysis module renv.lock file.

@danhtruong danhtruong marked this pull request as ready for review September 20, 2024 14:31
@jaclyn-taroni jaclyn-taroni requested review from jashapiro and removed request for jaclyn-taroni September 20, 2024 14:37
@jashapiro
Copy link
Member

Hi @danhtruong, thank you for your contribution!

The readme you have here looks good, but for this initial commit it would be good to have all of the module structure setup as you had it in #760. If you did run the module creation script, [create-analysis-module.py](https://github.com/AlexsLemonade/OpenScPCA-analysis/blob/main/create-analysis-module.py), you can commit all of the files that were created by it. If you did not run that script to create the analysis directory, or if you want to start fresh, I would probably do the following set of commands to set things up, run from the root of the repository (OpenScPCA-analysis/):

mv analyses/cell-type-dsrct analyses/cell-type-dsrct_orig
 ./create-analysis-module.py cell-type-dsrct --use-renv
mv analyses/cell-type-dsrct_orig/README.md analyses/cell-type-dsrct/README.md

Then you should be able to add and commit all of the files that the script created within analyses/cell-type-dsrct to this branch, as well as the skeleton workflow files that were created in .github/workflows.

Let me know if you have any questions about that process!

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.

Thank you for the update @danhtruong. It seemed from the readme that you were planning to use a few R packages, and for that we would really recommend setting up renv as well, but it is fine if you would like to leave that for a later PR.

I also made a suggestion that you include the command used for downloading the input data, as this makes it much easier for a later user to reproduce or update the analysis. Other than that, this should be in good shape as an initial PR.

Note that before merging we will need to make sure it is up-to-date with the current main branch. You can either do locally by merging in upstream/main or on GitHub with the "Update Branch" button, followed by pulling the changes back down to your local copy of the repo.

## Input files


The input is dependent on the output fles run from `download-data.py`. This generates `SingleCellExperiment` for DSRCT samples.
Copy link
Member

Choose a reason for hiding this comment

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

Can you include the command for downloading the data files specific to this analysis? I assume for this project that it would look like this?

Suggested change
The input is dependent on the output fles run from `download-data.py`. This generates `SingleCellExperiment` for DSRCT samples.
The input files are the `SingleCellExperiment` files for project SCPCP000013, which can be downloaded with the following command:
```
./download-data.py --project SCPCP000013
```

As you do not need all of these files for this project, you could specify the specific 7 samples instead, but that is really up to you, as the total project is not too large.

Added the tag

Co-authored-by: Joshua Shapiro <[email protected]>
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.

Okay looks good for the structure of the module! We will merge this in and then you can proceed on to the first stages of your actual analysis.

@jashapiro jashapiro merged commit 3ab9571 into AlexsLemonade:main Sep 20, 2024
2 checks passed
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