Skip to content
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

[NF] new workflow: FetchFlow #1843

Merged
merged 7 commits into from Jun 24, 2019

Conversation

@skoudoro
Copy link
Member

commented May 23, 2019

This PR adds a new workflow: FetchFlow. This command line is an easy way to grab our tutorial data via a command line.

Example:

$ dipy_fetch standford_hardi
$ dipy_fetch all
$ dipy_fetch all --out_dir=~/my_folder
$ dipy_fetch standford_hardi bundle_fa_hcp
$ dipy_fetch standford_hardi bundle_fa_hcp --out_dir=~/my_folder
@arokem arokem referenced this pull request May 23, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Jun 1, 2019

Codecov Report

Merging #1843 into master will increase coverage by 0.01%.
The diff coverage is 57.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1843      +/-   ##
==========================================
+ Coverage   84.28%   84.29%   +0.01%     
==========================================
  Files         117      117              
  Lines       14222    14297      +75     
  Branches     2245     2267      +22     
==========================================
+ Hits        11987    12052      +65     
- Misses       1718     1725       +7     
- Partials      517      520       +3
Impacted Files Coverage Δ
dipy/workflows/io.py 74.46% <57.77%> (-15.33%) ⬇️
dipy/tracking/local/localtracking.py 95.87% <0%> (-1.88%) ⬇️
dipy/tracking/utils.py 88.95% <0%> (+0.1%) ⬆️
dipy/workflows/tracking.py 96.34% <0%> (+0.39%) ⬆️
dipy/core/geometry.py 91.22% <0%> (+0.59%) ⬆️
dipy/data/fetcher.py 34.77% <0%> (+1.7%) ⬆️
@arokem
Copy link
Member

left a comment

Looks good. The only comment I have is a very minor comment about the docstring.

return importlib.import_module(module_path)

def run(self, data_names, out_dir=''):
"""Download files to folder and checks their md5 checksums.

This comment has been minimized.

Copy link
@arokem

arokem Jun 13, 2019

Member

Either "Downloads files to folder and checks their md5 checksums" or "Download files to folder and check their md5 checksums"

@arokem
Copy link
Member

left a comment

Looks great. Just a couple of questions.

dipy/workflows/io.py Show resolved Hide resolved
if __name__ == '__main__':
test_io_info()
test_io_fetch()
# test_io_info()

This comment has been minimized.

Copy link
@arokem

arokem Jun 20, 2019

Member

Why is this commented out?

This comment has been minimized.

Copy link
@skoudoro

skoudoro Jun 21, 2019

Author Member

I forgot to put it back after my test. Thanks !

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2019

Thank you for the review, just updated

Add ariel comment to be more explicit
Co-Authored-By: Ariel Rokem <arokem@gmail.com>
@arokem

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

OK. LGTM. Merging.

@arokem arokem merged commit dffd165 into nipy:master Jun 24, 2019

4 of 5 checks passed

codecov/patch 57.77% of diff hit (target 84.28%)
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
codecov/project 84.29% (+0.01%) compared to 8379bbd
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@skoudoro skoudoro deleted the skoudoro:fetch-flow branch Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.