-
Notifications
You must be signed in to change notification settings - Fork 3
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 CLI for coSMicQC
#34
Conversation
In chatting with @jenna-tomkinson, we discussed that getting a file-based output would potentially be more useful than text-based output. The filename could use the input filename as the basis for output filename, for example |
Decided to implement this within the same PR. |
Co-authored-by: Gregory Way <gregory.way@gmail.com>
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 am approving this PR since I do not see any issues in the code that looks to cause any bugs. I left a lot of comments with questions and suggestions for increasing clarity for a reader of the code. Nice job! 🎉
Thanks @jenna-tomkinson for the review! I've addressed your comments and will now merge this in. |
Description
This PR adds a CLI for
coSMicQC
through the use offire
. This work depends on changes from #32 . Along the journey towards these changes I discovered there were challenges in displayingpd.DataFrame
's (or relatedSCDataFrame
's) throughfire
CLI's. As a result, I added a patch inspired from work in google/python-fire#446 by reassigning the_PrintResult
function to match the contents of the fix. Once the fix is merged infire
we should remove the patch and rely onfire
for these changes. When/if these changes are merged, I'll create an issue to track this so we don't forget to take action here.Along with these changes some docs were fixed.
This should be reviewed and merged after #32
Closes #14
Closes #37
What kind of change(s) are included?
Checklist
Please ensure that all boxes are checked before indicating that this pull request is ready for review.