-
Notifications
You must be signed in to change notification settings - Fork 0
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
Included Pairwise Comparison Tool #1
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
This is a great starting point and I really like the idea of this becoming a tool for the Way lab and other labs! I am approving as a first PR, but I left a lot of comments with concerns to next steps that I would like to see and changes that would better align with Way Lab tool standards!
Please feel free to open issues (or not) on any of my comments for future PRs! I am also tagging @d33bs on this PR to ask for his feedback on packing this tool. He recently gave excellent feedback for CP_Paralell
which we just got off the ground. Please let me know if you have any questions on these comments @MattsonCam Great PR though!
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 job! I added a few comments throughout - please don't hesitate to reach out with any questions / comments.
Additionally - I noticed the repo is a private one. Typically this can come with added overhead for projects to somehow access the code (login / SSH key / etc). Consider making the work publicly accessible if possible (I'd defer to what you think is best).
README.md
Outdated
## Extensibility | ||
This tool is designed to compare feature between any two different rows within a tidy pandas dataframe. | ||
To include additional comparisons, you will need to introduce the functionality as a class and inherit from the Comparator class. |
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.
Consider providing a link to the specific file that includes the Comparator class.
PairwiseCompare modules from using pr comments and deprecation warnings. Modifies the example code to use data from the data submodule.
Added the NF1 cell dataset, and modified example comparisons code. Added init files for path management and poetry files for package/dependency management.
This is the basic setup of github actions to run pytest whenever merging, or on pull_request.
Thanks @d33bs and @MikeLippincott, I plan on making this installable and adding more details about the installation after merging this pr. Merging now! |
This pr includes the pairwise comparison tool, an example, and an introductory readme of the tool. Please let me know if you have a suggestion for an alternative location where this tool could be integrated, or if you have any other constructive feedback.