-
Notifications
You must be signed in to change notification settings - Fork 10
[WIP] New ebi submit [WIP] #704
base: master
Are you sure you want to change the base?
Conversation
python_code/ebi.py
Outdated
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.
should this drop #?
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.
Yeah it could, but the prep/sample templates I get from Gail never have comment lines starting with #
|
Thanks for the review @wasade, I'll implement these changes soon! |
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.
It would be nice to have descriptions for this and the subsequent Exceptions.
|
I like how the code is looking! I would like to hear what your plans are for testing the main EBI object, as it stands right now, I can see that it's acting a lot as a data formatter, hence I would be inclined to leave the submission of files to a separate object, would be curious to hear what others think. |
|
Agree, submit should be external
|
python_code/ebi.py
Outdated
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.
split_helper should be moved into here otherwise we have a circular dependency once the cmd line click'd interface imports EBISubmission
Almost there, but I thought I'd get this in now so that people can begin reviewing, it's kind of big.
Needs: