[MRG] Release+brainsprite cleanup #1856

kchawla-pi · 2018-11-10T17:44:10Z

Renames view_stat_map to view_img.
Updates whats new.
@jeromedockes CircleCI has failed twice on this now, because it couldn't read an image. Keep an eye and review this one.

This reverts commit 1b22d18

* 'master' of https://github.com/nilearn/nilearn: Update nilearn/image/tests/test_resampling.py Update nilearn/image/tests/test_resampling.py Update nilearn/image/tests/test_resampling.py Updated News section of the website nilearn.github.io Updated number of commits by contributer Changed version from 0.5.0a to 0.5.0b Added list of contributors for the alpha release Updated whats new & aAdded list of contributors for the beta release Simplified the test and fixed an issue previously introduced in the doc. Renamed extrapolation parament "fill_value". Added one test. Added support for fill value in resampling tools.

* 'master' of https://github.com/nilearn/nilearn: Typos Proper fix fix css Clean doc build

* 'master' of https://github.com/nilearn/nilearn: (23 commits) ENH: add new test to verify orthogonal filters updates changelog to not create circleci error update changes with new filter order in clean moving change under new release candidate standardize confounds after temporal filtering updates t_r documentation updates changelog improve error message in clean function prevent overwriting of t_r value Adds description text and Lindquist citation change order of data manipulation Apply temporal filter on confounds if needed Move band-pass filter before removing confounds Add reasoning to raising error + typo fix Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong add now missing t_r to signal tests Adds now missing t_r parameter to tests ...

* 'master' of https://github.com/nilearn/nilearn: (26 commits) Added a comment explaining the need for an alternate import MAINT: address deprecations in numpy ENH: add new test to verify orthogonal filters updates changelog to not create circleci error update changes with new filter order in clean moving change under new release candidate standardize confounds after temporal filtering updates t_r documentation updates changelog improve error message in clean function prevent overwriting of t_r value Adds description text and Lindquist citation change order of data manipulation Apply temporal filter on confounds if needed Move band-pass filter before removing confounds Add reasoning to raising error + typo fix Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong Improve error message of vol_to_surf when input is wrong ...

* 'master' of https://github.com/kchawla-pi/nilearn:

* 'master' of https://github.com/nilearn/nilearn: Allen atlases adds doc text to highlight mask and img mismatch fixes reference syntax change test to almost_equal reformats function according to feedback Checks that results from clean_img with and without mask is equal adds :func: and code-font to text remove niimg_3d check for mask_img (will be checked through apply_mask) FIX: add optional tag to input parameter add mask parameter to clean_img

@KamalakerDadi

…Dadi * 'master' of https://github.com/nilearn/nilearn: (106 commits) fix doc line too long. Added brief summary of the PR in whats_new.rst Removing deprecated argument anat_img at plot_prob_atlas Disable `value` when `colorbar=False` in `view_stat_map`. Improved error message when improper cut_coords is provided. Getting rid of _get_vmin_vmax, which is not used. Improving doc and comments, thanks to @KamalakerDadi's feedback. Added a `See Also` section to `view_stat_map`. Using `_utils.compat._encodebytes` instead of local import for python2/3 compatibility. Added `_encodbytes`, mapping to `base64.encodebytes` in python 3 and `base64.encodestring` in python 2. Some docs improvement, following up on @KamalakerDadi's suggestions. Use _utils.compat to deal with python2/3 compatibility. Bug fix in the doc. The screenshot for view_stat_map using brainsprite. Updated doc / screenshot from the papaya viewer to the brainsprite viewer. Updating brainsprite.js to fix a rounding error for `cut_coords` that resulted in broken value being reported in some situations. FiX: tests, 3 targets to 2, PEP8 stuff FIX: Passing meaningful error when y contains all 1s A bunch of improvements, mostly on naming and docstring. Added a test that `_get_vmin_vmax` works properly with NaNs in the data. ...

… screenshot - Renamed test_view_stat_map to test_view_img.

kchawla-pi · 2018-11-10T17:47:27Z

PR #1766 has already been merged yesterday

kchawla-pi · 2018-11-10T17:57:46Z

The log for the 3 failed buids, one from the [WIP] I closed, 2 from the new (this one).

I dunno why circleCI has disappeared.
circleci-3builds-log-release+brainsprite-cleanup.zip

KamalakerDadi · 2018-11-10T18:03:25Z

I dunno why circleCI has disappeared.

What did you made to reappear in #1855

codecov · 2018-11-10T18:10:34Z

Codecov Report

Merging #1856 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1856      +/-   ##
==========================================
+ Coverage   95.23%   95.24%   +0.01%     
==========================================
  Files         135      135              
  Lines       17320    17320              
==========================================
+ Hits        16495    16497       +2     
+ Misses        825      823       -2

Impacted Files	Coverage Δ
nilearn/plotting/tests/test_html_stat_map.py	`100% <100%> (ø)`	⬆️
nilearn/plotting/__init__.py	`82.6% <100%> (ø)`	⬆️
nilearn/plotting/html_stat_map.py	`98.65% <100%> (ø)`	⬆️
nilearn/datasets/tests/test_neurovault.py	`97.79% <0%> (+0.48%)`	⬆️

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 2b5fc46...86dfd01. Read the comment docs.

kchawla-pi · 2018-11-10T22:12:01Z

Didn't do anything to make it reappear or disappear It just happened on its own.

kchawla-pi · 2018-11-10T22:13:22Z

This same PR is running on my CircleCI (albeit failing)

KamalakerDadi · 2018-11-10T22:15:51Z

doc/plotting/index.rst

@@ -428,7 +428,7 @@ use :func:`view_markers`.
 :func:`view_img_on_surf`: Surface plot using a 3D statistical map::

    >>> from nilearn import plotting, datasets     # doctest: +SKIP
-    >>> img = datasets.fetch_localizer_button_task()['tmaps'][0]     # doctest: +SKIP
+    >>> img = datasets.fetch_localizer_button_task()['tmaps']     # doctest: +SKIP


Its not longer 'tmaps' but 'tmap'. Push a commit with this change. Perhaps it may reappear.

KamalakerDadi · 2018-11-10T22:26:07Z

For me I think you should raise a deprecation warning for two release cycles and remove view_stat_map completely.

kchawla-pi · 2018-11-10T22:34:30Z

Wait, I don't understand. Didn't I replace all references to view_stat_map with 'view_img' in this PR?
Also, view_stat_map was introduced in 0.5.0 alpha. Do we issue deprecation warnings for things introduced in alphas of the current build?

kchawla-pi · 2018-11-10T22:38:09Z

Phew CircleCI is back. now when it fails we can diagnose it.

KamalakerDadi · 2018-11-10T22:41:42Z

Didn't I replace all references to view_stat_map with 'view_img' in this PR?

what do you mean ?

Do we issue deprecation warnings for things introduced in alphas of the current build?

Check in the merged PR. It is discussed there

GaelVaroquaux · 2018-11-11T07:56:34Z

We never released something that was not a beta or alpha with view_stats_map. So we don't need to go through a depreciation cycle. We should just mention things clearly in the release notes.

GaelVaroquaux

Has view_img been added or modified in doc/module/reference.rst?

kchawla-pi · 2018-11-11T15:55:50Z

Yes

…

On Sun, Nov 11, 2018, 09:02 Gael Varoquaux ***@***.*** wrote: ***@***.**** commented on this pull request. Has view_img been added or modified in doc/module/reference.rst? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1856 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH0rxJeeW0Lue9JhwqtPxQ7owpvF2jkNks5ut9mygaJpZM4YYF3a> .

…r view_img

kchawla-pi · 2018-11-12T21:18:55Z

@GaelVaroquaux Updated whats_new.rst . Review it. If I don't see any problems, I will merge this to master Tuesday evening, and then release.

kchawla-pi · 2018-11-12T23:19:52Z

@GaelVaroquaux shouldn't we remove the warning from view_stat_map, now view_img, that it can change without warning and not to use it in production environments?

pbellec · 2018-11-13T06:10:52Z

@kchawla-pi just seeing this. I posted a PR to finish the work we discussed on view_stat_map (#1858). I don't think it will conflict with this one. I am busy all of Tuesday, but I will try to merge this PR into the other one on Wednesday.

GaelVaroquaux · 2018-11-13T07:02:16Z

@GaelVaroquaux shouldn't we remove the warning from view_stat_map, now view_img, that it's can change without warning and not to use it in production environments?

Yes

kchawla-pi · 2018-11-13T18:47:52Z

@GaelVaroquaux There are more commits here from the previous release than there should be. I am not sure how they got here, I created this branch from master after merging pbellec's PR.
How do I fix that? Rebase?

kchawla-pi · 2018-11-13T18:49:56Z

Seems the warning has already been removed by someone else in one of the PRs. Move along nothing to see here.

KamalakerDadi · 2018-11-15T20:35:15Z

There is a white spot appears on the screen shot. Could you correct that ?

KamalakerDadi · 2018-11-15T20:39:48Z

Also, there is value displaying in the screenshot. Probably, this needs to be corrected as well. A screenshot without value.

KamalakerDadi · 2018-11-15T20:46:50Z

doc/whats_new.rst

+  eg for use in a notebook.
+
+- New functions :func:`nilearn.plotting.view_surf` and
+  :func:`nilearn.plotting.view_surf` and


view_surf is repeated for another time.

KamalakerDadi · 2018-11-15T20:48:23Z

doc/whats_new.rst

+  :func:`nilearn.plotting.view_img_on_surf` for interactive visualization of
+  maps on the cortical surface in a web browser.
+
+- New functions :func:`nilearn.plotting.view_connectome` and


A mention of "Interactive" is missing here in this statement.

KamalakerDadi · 2018-11-15T20:50:03Z

doc/whats_new.rst

+- New function :func:`nilearn.plotting.view_img` for interactive
+  visualization of volumes with 3 orthogonal cuts.
+
+:Note: :func:`nilearn.plotting.view_img` was `nilearn.plotting.view_stat_map` in preview builds.


I did understand what you mean by preview here ?

Alpha and Beta builds

Are they builds or releases ?

Releases. Will clarify.

KamalakerDadi · 2018-11-15T20:58:51Z

doc/whats_new.rst

+----------
+
+:func:`nilearn.plotting.view_img` (formerly `nilearn.plotting.view_stat_map` in
+NiLearn 0.5.0 pre-release versions) generates significantly smaller notebooks


I used to write L capital in Nilearn. But then I have been taught that I should not write L capital.

KamalakerDadi · 2018-11-15T21:00:08Z

doc/whats_new.rst

+---------------
+
+ Default value of `t_r` in :func:`nilearn.signal.clean` and
+ :func:`nilearn.image.clean_img` is now 2.5


Are you sure it is 2.5. By looking at the documentation it is None.

I repeated what was posted in an earlier What's New (for the alpha or Beta release). Since it had not been brought up again I assumed that it wasn't changed again. My mistake. Will check.

kchawla-pi · 2018-11-15T21:01:06Z

Also, there is value displaying in the screenshot. Probably, this needs to be corrected as well. A screenshot without value.

Also it is still showing view_stat_map instead of view_img. Thanks.

KamalakerDadi · 2018-11-15T21:01:39Z

Also it is still showing view_stat_map instead of view_img. Thanks.

Good catch.

KamalakerDadi · 2018-11-15T21:07:29Z

doc/whats_new.rst

+
+**LOTS of changes and improvements. Detailed change list follows.**
+
+0.5.0 rc


Do we need this ?

The enthusiasts who like to install alpha and beta builds would want to know what changed from beta to stable, which is covered under rc. Early testers often hunger for extra details in the change log. Doing these things helps maintain their ongoing engagement, and gives us a steady pool of alpha-beta testers.

The enthusiasts, passionate about Nilearn but not beta testers upgrading from 0.4.2 to 0.5.0 would like to quickly see all the best new things improved and to try out. For that I have an overarching summary, under 0.5.0 .

For many remaining users, they won't care if we don't include a What's New, so this isn't for them at all.

maybe LOTS -> Lots

KamalakerDadi · 2018-11-15T21:14:11Z

nilearn/plotting/html_stat_map.py

@@ -329,7 +329,7 @@ def _get_cut_slices(stat_map_img, cut_coords=None, threshold=None):
    return cut_slices


-def view_stat_map(stat_map_img, bg_img='MNI152', cut_coords=None,
+def view_img(stat_map_img, bg_img='MNI152', cut_coords=None,
                  colorbar=True, title=None, threshold=1e-6, annotate=True,


Could you please check this ? Indentation missing.

…is PR

jeromedockes · 2018-11-16T12:31:15Z

LGTM

KamalakerDadi · 2018-11-16T12:47:39Z

I don't like these kind of import patterns in the screen shots and it is not consistent with demo code. I don't see the necessity of it.

from nilearn import (datasets,
                     plotting,
                    )

kchawla-pi · 2018-11-16T12:55:21Z

That's the official PEP8 recommendation for multiple imports from a single from source.
I don't think it will confuse the users.
I do think this a good way to showcase small improvements in coding practices.
I am not attached to this, if it should be changed, I'll change it.

- Changed plotely_surface_plot_screenshot.png to show the right side of the right hemisphere with more positive activations than the earlier left with more negative activations. - Switched to one import per import statement for stylistic consistency. - Removed the highlight in a sentence in whats_new.rst. Too much.

kchawla-pi added 13 commits October 16, 2018 20:39

Added list of contributors for the beta release

1b22d18

Revert "Added list of contributors for the beta release"

0a68af0

This reverts commit 1b22d18

Merge branch 'master' after some typo & websites css fixes

69aeca7

* 'master' of https://github.com/nilearn/nilearn: Typos Proper fix fix css Clean doc build

Merge branch 'master' of https://github.com/kchawla-pi/nilearn

d6e2360

* 'master' of https://github.com/kchawla-pi/nilearn:

Renamed plotting.view_stat_map to plotting.view_img, fixed an example…

de86caa

… screenshot - Renamed test_view_stat_map to test_view_img.

Updated whats_new.rst

3e5f3ec

Improved order of entries in whats_new.rst

32b8687

Updated whats_new.rst & some doc changes

ba05304

jeromedockes mentioned this pull request Nov 10, 2018

switch from papaya to brainsprite in plotting.view_stat_map #1766

Merged

KamalakerDadi reviewed Nov 10, 2018

View reviewed changes

Replaced ['tmaps'] with ['tmap'] ins code and screenshot

7ed5e1b

GaelVaroquaux reviewed Nov 11, 2018

View reviewed changes

Added contributors for Beta-RC release, renamed screenshot file as pe…

7701f9e

…r view_img

Corrected a name in list of contributors

fee3829

kchawla-pi changed the title ~~Release+brainsprite cleanup~~ [MRG] Release+brainsprite cleanup Nov 15, 2018

kchawla-pi added 2 commits November 15, 2018 14:49

Merge branch 'master' into release+brainsprite-cleanup

0bc0afc

Updated contributor commits count

b9f15f0

KamalakerDadi reviewed Nov 15, 2018

View reviewed changes

kchawla-pi added 3 commits November 16, 2018 11:00

Incorporated most of the feedback from Kamalaker's first review of th…

d97cf97

…is PR

Removed the erring comma in view_img parameters, fixed 2 example images

ddd3def

Updated commits count for contributors

bcb3e26

kchawla-pi merged commit e6e7df0 into nilearn:master Nov 16, 2018

kchawla-pi deleted the release+brainsprite-cleanup branch November 28, 2018 08:55


		LOTS of changes and improvements. Detailed change list follows.

		0.5.0 rc

[MRG] Release+brainsprite cleanup #1856

[MRG] Release+brainsprite cleanup #1856

Conversation

kchawla-pi commented Nov 10, 2018 • edited

kchawla-pi commented Nov 10, 2018

kchawla-pi commented Nov 10, 2018

KamalakerDadi commented Nov 10, 2018

codecov bot commented Nov 10, 2018 • edited

Codecov Report

kchawla-pi commented Nov 10, 2018

kchawla-pi commented Nov 10, 2018 • edited

Choose a reason for hiding this comment

KamalakerDadi commented Nov 10, 2018

kchawla-pi commented Nov 10, 2018

kchawla-pi commented Nov 10, 2018

KamalakerDadi commented Nov 10, 2018

GaelVaroquaux commented Nov 11, 2018

GaelVaroquaux left a comment

Choose a reason for hiding this comment

kchawla-pi commented Nov 11, 2018 via email

kchawla-pi commented Nov 12, 2018

kchawla-pi commented Nov 12, 2018 • edited

pbellec commented Nov 13, 2018

GaelVaroquaux commented Nov 13, 2018 via email

kchawla-pi commented Nov 13, 2018 • edited

kchawla-pi commented Nov 13, 2018

KamalakerDadi commented Nov 15, 2018

KamalakerDadi commented Nov 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kchawla-pi commented Nov 15, 2018

KamalakerDadi commented Nov 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromedockes commented Nov 16, 2018

KamalakerDadi commented Nov 16, 2018

kchawla-pi commented Nov 16, 2018

kchawla-pi commented Nov 10, 2018 •

edited

codecov bot commented Nov 10, 2018 •

edited

kchawla-pi commented Nov 10, 2018 •

edited

kchawla-pi commented Nov 12, 2018 •

edited

kchawla-pi commented Nov 13, 2018 •

edited