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

Restore Localizer fetchers interface for backwards compatibility #2182

Merged

Conversation

kchawla-pi
Copy link
Collaborator

@kchawla-pi kchawla-pi commented Oct 16, 2019

Fixes #2181

Making fetch_localizer_button_task & fetch_localizer_calculation_task backwards compatible.

  • TODO: fetch_localizer_calculation_task()
  • TOFIX: Example plot_dim_plotting.py is failing here & in pre PR 2097.

 - TOFIX: Example plot_dim_plotting.py is failing here & in pre PR 2097.
@codecov
Copy link

codecov bot commented Oct 16, 2019

Codecov Report

Merging #2182 into master will decrease coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2182      +/-   ##
==========================================
- Coverage   92.34%   92.18%   -0.16%     
==========================================
  Files         149      149              
  Lines       18867    18868       +1     
  Branches     2302     2299       -3     
==========================================
- Hits        17423    17394      -29     
- Misses        924      952      +28     
- Partials      520      522       +2
Impacted Files Coverage Δ
nilearn/datasets/tests/test_func.py 99.41% <100%> (ø) ⬆️
nilearn/datasets/func.py 81.42% <100%> (+0.42%) ⬆️
nilearn/conftest.py 47.36% <0%> (-52.64%) ⬇️
nilearn/plotting/__init__.py 78.26% <0%> (-17.4%) ⬇️
nilearn/input_data/nifti_masker.py 87.6% <0%> (-4.96%) ⬇️
nilearn/_utils/compat.py 43.33% <0%> (-3.34%) ⬇️
nilearn/__init__.py 68.57% <0%> (-2.86%) ⬇️
nilearn/decoding/fista.py 87.65% <0%> (-2.47%) ⬇️
nilearn/regions/parcellations.py 94.28% <0%> (-1.91%) ⬇️
nilearn/image/tests/test_image.py 98.36% <0%> (-1.1%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc6a019...7a9605c. Read the comment docs.

@kchawla-pi
Copy link
Collaborator Author

@jeromedockes I didn't spot any obvious changes in fetch_localizer_calculation_task. Would you point me in what's different?

@jeromedockes
Copy link
Member

jeromedockes commented Oct 17, 2019 via email

@kchawla-pi
Copy link
Collaborator Author

I am comparing fetch_localizer_calculation_task from e7b118b7 and they both have n_subjects and list as return.

@jeromedockes
Copy link
Member

jeromedockes commented Oct 17, 2019 via email

@kchawla-pi
Copy link
Collaborator Author

Yeah I was surprised to see that as well.

@jeromedockes
Copy link
Member

with the stable release the returned dicts have:

calculation: cmaps: <class 'list'>, ext_vars: <class 'numpy.recarray'>, description: <class 'bytes'>
button: description: <class 'bytes'>, tmap: <class 'str'>, anat: <class 'str'>

@jeromedockes
Copy link
Member

and the parameters:

fetch_localizer_button_task(data_dir=None, url=None, verbose=1)
fetch_localizer_calculation_task(n_subjects=1, data_dir=None, url=None, verbose=1)

@jeromedockes
Copy link
Member

maybe they should both have:

  * cmaps: list of 1 image (useless, here for backwards compatibility with stable fetch_localizer_calculation_task)
  * cmap: nifti image
  * tmap: nifti image
  * anat: nifti image

this way it is backward compatible but both functions return the same things.

@kchawla-pi
Copy link
Collaborator Author

maybe they should both have:

That is a different PR however. I will open a new PR for that.
Anything else here for the backward compatibility?

@kchawla-pi kchawla-pi merged commit 58ded60 into nilearn:master Oct 21, 2019
@kchawla-pi kchawla-pi deleted the fix-brainomics-pr2097-regression branch October 21, 2019 13:17
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this pull request Oct 21, 2019
* 'master' of github.com:nilearn/nilearn:
  Making fetch_localizer_button_task backwards compatibile (nilearn#2182)
kchawla-pi added a commit to kchawla-pi/nilearn that referenced this pull request Oct 24, 2019
…te-req-dev

* 'master' of https://github.com/nilearn/nilearn: (336 commits)
  Release Nilearn 0.6.0 alpha (nilearn#2164)
  Making fetch_localizer_button_task backwards compatibile (nilearn#2182)
  [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)
  ...
kchawla-pi added a commit to illdopejake/nilearn that referenced this pull request Nov 9, 2019
…nt_modify_fetch_dev

* 'master' of https://github.com/nilearn/nilearn:
  Verbose doc building to ease tracking of progress & diagnose stalls (nilearn#2203)
  New conda env is created once conda path has been activated
  Conda environment is created for full-builds
  Refactor CircleCI config for reduced redundancy (nilearn#2204)
  Installation should fail on Python < 3.5 (nilearn#2198)
  [MRG] Add get data function (nilearn#2172)
  fix error when colorscale given boolean array (nilearn#2193)
  remove is_valid filter (nilearn#2169)
  Moved new entries to next release
  FIX: marker size issue in plot_connectome nilearn#2185 (nilearn#2186)
  Updated Appveyor status badge
  Run tests on local Windows machines & Azure Pipelines (nilearn#2191)
  Updated requirements list for devs (nilearn#2190)
  Update azure-pipelines.yml for Azure Pipelines
  Release Nilearn 0.6.0 alpha (nilearn#2164)
  Making fetch_localizer_button_task backwards compatibile (nilearn#2182)
  [DOC] Update whats_new to reference nilearn#2013 (Merging of several examples) (nilearn#2183)
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

Successfully merging this pull request may close these issues.

restore localizers interface
3 participants