ENH: Added a datagrabber capable of grabbing files from an SSH access #844
Conversation
|
Instead of just copying the DataGrabber code, why not use inheritance? |
|
I totally Agree with @mwaskom and my last commit goes in that direction. Further work on the inheritance part would need a refactoring on the DataGrabber class I think. Any suggestions on how to setup doctests that run? e.g. an SSH server that we could use for free to make them run? |
| """ | ||
| input_spec = SSHDataGrabberInputSpec | ||
| output_spec = DynamicTraitedSpec | ||
| _always_run = True |
There was a problem hiding this comment.
Do you really want this to always run? It seems like a pretty expensive interface.
There was a problem hiding this comment.
You are absolutely right! Thanks
|
@mwaskom None of the other interfaces with optional dependencies (XNAT, SQL) checks for the support libraries in the init method. They do it, implicitly, in the _list_outputs. This difference provokes that my module does not successfully complete the doctests. So I'm changing the exception rethrow to a waning in init and to an exception in list_outputs |
…n if the library is not there
|
@mwaskom could you please help me shaping up the PR? The 2.6 test is failing due to a library I haven't even touched and as I maintained the testing infrastructure of the XNAT modules, the coverage has decreased. What do I need to get it to a merge-possible state? Thanks |
|
It looks like the failure on 2.6 is because of a set literal, which aren't supported on that version. |
|
@mwaskom yup, but I didn't touch the file where that is happening ( ERROR: Failure: SyntaxError (invalid syntax (model.py, line 407)) That's why I'm puzzled |
|
I can see why that would be confusing :). @satra this is downloading |
|
@demianw seems to be corrected now in master I fetched upstream and then merged to my branches. Now they pass the tests |
|
Worked! Thanks @oesteban |
|
@demianw thanks for your hard work! Could you add a mention of this new feature in CHANGES file? I think that's the last thing remaining before the merge. |
|
@chrisfilo voila |
ENH: Added a datagrabber capable of grabbing files from an SSH access
|
Yay |
This is a simple extension to the default DataGrabber. Tried to copy the XNAT data grabber for the testing but realised that there is no actual testing for data grabbers that grab data over a network
Comments welcome!