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

Update readme with GHA instructions and more detail #52

Merged
merged 10 commits into from
May 29, 2024

Conversation

jashapiro
Copy link
Member

Closes #32

This PR adds more detail about running the openscpca-nf workflow, including most notably the discussion of running via GHA.

Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

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

I left a few comments and questions that I think need to be clarified.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated

```bash
nextflow run AlexsLemonade/OpenScPCA-nf -profile batch
```

For most use cases you will want to specify an output directory other than the default using the `--results_bucket` argument.
Copy link
Member

Choose a reason for hiding this comment

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

What are most cases? When would you want to use the default bucket?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default bucket is currently the results release bucket. I expect some of this to change when we get to #44, but for now it is what it is. Writing this sentence was actually what led me to file #44; I expect to add a production profile that sets the output buckets and then lock them down a bit more to avoid accidental edits, with some kind of staging location the default, but we don't have all of that set up yet.

Suggested change
For most use cases you will want to specify an output directory other than the default using the `--results_bucket` argument.
For most use cases you will want to use the `--results_bucket` argument to avoid writing to the default output bucket.

```bash
nextflow run AlexsLemonade/OpenScPCA-nf -profile batch --results_bucket {OUTDIR}
```

The workflow also has a couple of entry points other than the main workflow, for testing and creating simulated data.
Copy link
Member

Choose a reason for hiding this comment

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

I might make a new header for talking about entry points.

README.md Outdated Show resolved Hide resolved
README.md Outdated
nextflow run AlexsLemonade/OpenScPCA-nf -profile batch -entry simulate --sim_pubdir {SIMDIR}
```

To run the workflow with simulated data, you can add the `simulated` profile.
Copy link
Member

Choose a reason for hiding this comment

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

This is different then running the simulated entry point right? I would explicitly state that/ make a new header.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is different. One generates the simulated data, the other uses it. I didn't like the overlap, but since one is an entry point and one is a profile, I thought it might be okay.

jashapiro and others added 2 commits May 29, 2024 16:05
Co-authored-by: Ally Hawkins <54039191+allyhawkins@users.noreply.github.com>
Co-authored-by: Ally Hawkins <54039191+allyhawkins@users.noreply.github.com>
Copy link
Member Author

@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.

Thanks for the feedback! I took your suggestion, and I will also add a couple more headers, perhaps with some mild reorganization.

README.md Outdated Show resolved Hide resolved
README.md Outdated

```bash
nextflow run AlexsLemonade/OpenScPCA-nf -profile batch
```

For most use cases you will want to specify an output directory other than the default using the `--results_bucket` argument.
Copy link
Member Author

Choose a reason for hiding this comment

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

The default bucket is currently the results release bucket. I expect some of this to change when we get to #44, but for now it is what it is. Writing this sentence was actually what led me to file #44; I expect to add a production profile that sets the output buckets and then lock them down a bit more to avoid accidental edits, with some kind of staging location the default, but we don't have all of that set up yet.

Suggested change
For most use cases you will want to specify an output directory other than the default using the `--results_bucket` argument.
For most use cases you will want to use the `--results_bucket` argument to avoid writing to the default output bucket.

README.md Outdated
nextflow run AlexsLemonade/OpenScPCA-nf -profile batch -entry simulate --sim_pubdir {SIMDIR}
```

To run the workflow with simulated data, you can add the `simulated` profile.
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is different. One generates the simulated data, the other uses it. I didn't like the overlap, but since one is an entry point and one is a profile, I thought it might be okay.

Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor wording changes.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
jashapiro and others added 2 commits May 29, 2024 18:49
Co-authored-by: Ally Hawkins <54039191+allyhawkins@users.noreply.github.com>
Co-authored-by: Ally Hawkins <54039191+allyhawkins@users.noreply.github.com>
@jashapiro jashapiro merged commit b4940c7 into main May 29, 2024
2 checks passed
@jashapiro jashapiro deleted the jashapiro/readme-gha branch May 30, 2024 20:58
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.

Update documentation with entrypoints, profiles, and triggers
2 participants