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

Add methylation summary workflow #307

Closed

Conversation

sickler-alex
Copy link

@sickler-alex sickler-alex commented Jan 18, 2023

Purpose/implementation Section

What GitHub issue does your pull request address?

d3b-center/ticket-tracker-OPC#456

Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.

Review workflow and corresponding tools. None of the R or python scripts were changed so this module should still run the same from the shell script and in CI.

Which areas should receive a particularly close look?

Is there anything that you want to discuss further?

Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?

No, will need to run with the full data set.

Results

What types of results are included (e.g., table, figure)?

Cavatica Test

What is your summary of the results?

Reproducibility Checklist

  • The dependencies required to run the code in this pull request have been added to the project Dockerfile.
  • [] This analysis has been added to continuous integration. - No. this is being addressed by a separate ticket.

Documentation Checklist

  • This analysis module has a README and it is up to date.
  • This analysis is recorded in the table in analyses/README.md and the entry is up to date.
  • The analytical code is documented and contains comments.

Copy link

@migbro migbro left a comment

Choose a reason for hiding this comment

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

Some minor comments I think

- class: InlineJavascriptRequirement
- class: ResourceRequirement
ramMin: ${return inputs.ram * 1000}
coresMin: 4

Choose a reason for hiding this comment

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

why 4 and not 1 ? does this script requires 4 cores?

Copy link
Author

Choose a reason for hiding this comment

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

It probably doesn't need 4. When I run it, I'll try with 1.

@sickler-alex sickler-alex force-pushed the feature/methylation-summary-workflow branch from 30f6cdf to c9e1499 Compare January 23, 2023 15:32
@sickler-alex sickler-alex changed the title 🚧 add first commit of 03 tool Add methylation summary workflow Jan 25, 2023
shellQuote: false
valueFrom: |-
./.git
mkdir -p ./.git/objects
Copy link
Author

Choose a reason for hiding this comment

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

These are created because the scripts in open ped can try to find the data and module dir based on searching the directory tree until it finds the .git directory which is created at the root dir of a git repo. In R, the command just looks for a directory containing a directory called .git however, GitPython checks for a few more directories and files that are also created by git. So the tools are creating these fake git directories so GitPython can find the corret root dir.

@sickler-alex sickler-alex force-pushed the feature/methylation-summary-workflow branch from 003d3b1 to d0d3fb6 Compare January 25, 2023 18:02
@jharenza
Copy link
Collaborator

jharenza commented Jun 4, 2023

Hey @ewafula and @sickler-alex. Given the state of this project, should we close this PR?

@jharenza
Copy link
Collaborator

Going to close this as MTP-specific and not needed right now

@jharenza jharenza closed this Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants