-
Notifications
You must be signed in to change notification settings - Fork 558
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
Implementation of RFC64: 'mutational signatures' in the backend #9913
Implementation of RFC64: 'mutational signatures' in the backend #9913
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice start, I proposed some minor restructuring to make the code more intuitive.
I feel like we should move the temposig outside of the script, and let the user give an installation directory.
Another thing, I discourage the removal of directories in the script. Just make clear in the names of the generated tmp files that they are temporary.
3d918d6
to
dd4d46d
Compare
dd4d46d
to
1ef34cf
Compare
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Matthijs,
Some code suggestions and a few spots where I think we can improve on clarity, structure or efficiency, as indicated.
It would be nice to add these scripts in a subfolder mutational_signatures
to keep this together.
Also, how can we work with the SigProfiler
package in the Docker image?
Implementation moved to cbioportal/datahub-study-curation-tools: PR#48 |
Implementation of RFC#64 in the backend
Describe changes proposed in this pull request:
TODO's:
Checks