-
Notifications
You must be signed in to change notification settings - Fork 24
Code cleanup and refactoring #79
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
Conversation
Refactor as needed Rename Dataset.py to lower case
It was making the module import due to the name collision
No longer needed now that SPRAS uses Docker for pathway reconstruction algorithms
Good ideas in the prototype but current design has diverged considerably. No immediate plans to add BowTieBuilder
Previously space-separated
Follow Python naming conventions https://github.com/naming-convention/naming-convention-guides/tree/master/python
|
The scope of this pull request is large enough so I'm going to stop here. This closes #76 even though it does not add pre-commit hooks. Reviewing the commit messages is the best way to get an overview of the changes made. To summarize:
@ntalluri can you please review these changes? Some affect your ML code, and you also have a good sense of how all of the SPRAS components work together. @annaritz you don't need to do a full review but could quickly browse the summary above and commit messages to make sure you don't object to any changes. |
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.
I had to update the test files to work with the version 2 code so directory names are different to make it work with the updated ml code
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.
I can manually tab separate these files once these changes are added
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.
can keep this, I can just update the names to work with the new code that is used in version 2
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.
I can update the tab seperated part on my end since I had to change names that are in this file for version2
Work in progress pull request for #76. I'll edit this to add details when it's ready for review.