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

Bug in _uncompress_file #2029

Closed
SylvainTakerkart opened this issue Apr 18, 2019 · 6 comments
Closed

Bug in _uncompress_file #2029

SylvainTakerkart opened this issue Apr 18, 2019 · 6 comments

Comments

@SylvainTakerkart
Copy link
Contributor

Hi!
(...still nilearn-sprinting in the train back to Marseille ;) )

I found a bug when trying to download .tar.gz archive files from osf.

I got an error when using _fetch_files:

from nilearn.datasets.utils import _fetch_files
opts = {'uncompress': True}
data_dir = '~/nilearn_data'
url = 'https://osf.io/vym9k/download'
_fetch_files(data_dir, [('sphere_right.gii', url, opts)])

After digging, I realized the problem is in _uncompress_file: the temporary downloaded file is actually called 'download' on disk (because of the url that's given by osf), and _uncompress_file does not extract the files if the archive was a .tar.gz one (it does not return an error, and it erases the 'download' file)

Note that:

  • if the archive is a zip file, everything works... (as in the fetch_icbm152_2009 function); the temporary file is also called 'download' on disk, but _uncompress_file handles it properly
  • if the file on disk is actually called 'whatever.tar.gz' and you run _uncompress_file on it, it also handles it properly

Soooooooooo, to reproduce the problem, the easiest is that:

  1. you type https://osf.io/vym9k/download in your browser to get the file
  2. you rename the file manually in your unix shell
> mv sphere_mesh.tar.gz download
  1. you try to use _uncompress_file on it
from nilearn.datasets.utils import _uncompress_file
_uncompress_file('download')

It will: 1. not extract the files in the archive, 2. erase the 'download' file, 3. not return an error.

I hope this is clear enough...

Sylvain

@kchawla-pi
Copy link
Collaborator

kchawla-pi commented Apr 19, 2019

@SylvainTakerkart asked me to replace on OSF the two .tar.gz files with the .zip files and send the two new links.
On it.

@kchawla-pi
Copy link
Collaborator

InterTVA: https://osf.io/aekp7/download
sphere mesh: https://osf.io/b79fy/download

@SylvainTakerkart
Copy link
Contributor Author

Thanks! I can then move forward with writing my fetchers (just created a PR for the sphere meshes)...

But this bug will still need to be looked at at some point...

@kchawla-pi
Copy link
Collaborator

kchawla-pi commented Apr 19, 2019

File an issue, with all the details.

Edit: Sorry I thought this was a PR.

AlexandreAbraham added a commit to AlexandreAbraham/nilearn that referenced this issue Aug 27, 2019
@AlexandreAbraham
Copy link
Contributor

Hello,

Indeed, there was a bug in the function. I created a PR to fix this:
#2126

Although, this kind of thing has been taken care of at _fetch_files level. Indeed, there is an opt to move a file after downloading (it's a bit of a hidden feature because it's a bit hacky). If you use

opts = {'uncompress': True, 'move': 'download.tgz'}

Then you should be fine because fetch_files with download your file, move it from download to download.gz and then _uncompress_file should work ok.

I am the unfaithful author of this evilish machinery, do not hesitate to mail me if you have that kind of issue.

@SylvainTakerkart
Copy link
Contributor Author

SylvainTakerkart commented Aug 28, 2019

Hi Alexandre, thanks for your message!
I have not contributed much to nilearn until now, but with the small things I've done that needed to setup some new data fetchers, I have to admit I have been struggling with using this machinery (I'm happy you self-caracterize it as evilish, this was my feeling also, haha ;) ), sometimes even trying to copy-paste its behavior from existing code... Is there any kind of documentation or tutorial available? That would be super useful!
Thanks,
Sylvain

kchawla-pi added a commit to AlexandreAbraham/nilearn that referenced this issue Sep 30, 2019
…clean

* 'master' of https://github.com/nilearn/nilearn: (2100 commits)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  added whats_new
  Added test for labels=None
  More explicit doc string
  Test for right length of labels
  Added tests for empty list case and labels as ndarray
  ensure labels is a list, not an ndarray
  Change such that empty lists are recognized as False
  Undid some documentation changes as their reason is not understood
  Replaced conda latest version with 4.6.* pending replacement
  Removed omit sections (nothing to omit, it would seem)
  Corrected docstring for bg_img in plot_stat_map
  Removed omit=*test* to fix error during coverage reporting
  ...

# Conflicts:
#	nilearn/tests/test_signal.py
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this issue Oct 8, 2019
…encoding

* 'master' of github.com:nilearn/nilearn: (228 commits)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  added whats_new
  Added test for labels=None
  More explicit doc string
  Test for right length of labels
  Added tests for empty list case and labels as ndarray
  ensure labels is a list, not an ndarray
  ...
kchawla-pi added a commit to KamalakerDadi/nilearn that referenced this issue Oct 9, 2019
…s_brain_orientation

* 'master' of https://github.com/nilearn/nilearn: (116 commits)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  added whats_new
  ...

# Conflicts:
#	doc/whats_new.rst
kchawla-pi added a commit to glemaitre/nilearn that referenced this issue Oct 9, 2019
…lot_connectome_strength

* 'master' of https://github.com/nilearn/nilearn: (272 commits)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  ...

# Conflicts:
#	examples/03_connectivity/plot_sphere_based_connectome.py
kchawla-pi added a commit to KamalakerDadi/nilearn that referenced this issue Oct 9, 2019
…ounds_for_connectome

* 'master' of https://github.com/nilearn/nilearn: (227 commits)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  ...
kchawla-pi added a commit to KamalakerDadi/nilearn that referenced this issue Oct 9, 2019
…_roi_contours

* 'master' of https://github.com/nilearn/nilearn: (227 commits)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  ...
kchawla-pi added a commit to illdopejake/nilearn that referenced this issue Oct 12, 2019
…nt_modify_fetch_dev

* 'master' of https://github.com/nilearn/nilearn: (228 commits)
  [MRG] Nans in view connectome (nilearn#2166)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  ...

# Conflicts:
#	nilearn/datasets/func.py
kchawla-pi added a commit to KamalakerDadi/nilearn that referenced this issue Oct 14, 2019
…ecomposition

* 'master' of https://github.com/nilearn/nilearn: (348 commits)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  Update Schaefer parcelation to v0.14.3 (nilearn#2138)
  MAINT: Future-compatible cmap reversal (nilearn#2131)
  fix openmp crash (nilearn#2140)
  change nose to pytest on appveyor (nilearn#2130)
  adding fix to whatsnew
  fix wrong urls in nki dataset
  Made Flake8 happy
  ...

# Conflicts:
#	doc/connectivity/resting_state_networks.rst
#	doc/whats_new.rst
#	examples/03_connectivity/plot_canica_analysis.py
kchawla-pi added a commit to pausz/nilearn that referenced this issue Oct 18, 2019
…smooth-image

* 'master' of https://github.com/nilearn/nilearn: (114 commits)
  [DOC] Update whats_new to reference nilearn#2013 (Merging of several examples) (nilearn#2183)
  Removed sub-example due to unfit for lasso dataset - unstable float values (nilearn#2177)
  Fix Flake8 errors overlooked when merging PR nilearn#2028 (Plot connectome strength) (nilearn#2174)
  ENH: add connectome strength plot (nilearn#2028)
  Fix cache mixin tests (nilearn#2161)
  Do not fail if metadata cannot be updated for an image (nilearn#2167)
  [MRG] Nans in view connectome (nilearn#2166)
  [MRG] FIX: orientation problem with plot_glass_brain (nilearn#1888)
  Import HTMLDocument in its original module to preserve backwards compatibility (nilearn#2162)
  Fixing malfunctioning allowed-failure section in Travis (nilearn#2160)
  Update Brainomics fetcher (nilearn#2097)
  Remove inplace modification in signal.clean (nilearn#2125)
  Iter age group prediction example (nilearn#2063)
  Expose bg_img, vmin and vmax in plot_img signature (nilearn#2157)
  Replac conda with pip in TravisCI setup (nilearn#2141)
  Core devs doc and add @emdupre (nilearn#2151)
  Add check for vmin, vmax in plot_surf_roi (nilearn#2052)
  [ENH] Initial visual reports (nilearn#2019)
  Renamed test to deduplicate name (nilearn#2144)
  Fixes nilearn#2029 Handle gzip files without extensions (nilearn#2126)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants