[MRG] ReNA_mrg

[ja-che]

Continue #1336

[ja-che]

#1336 (comment)

[ja-che]

Hello,

@GaelVaroquaux or @bthirion
I would need a review on that, please.
Should we move ReNA in another module folder ?

Thanks

[GaelVaroquaux]

Can you add ReNA to the end of examples/03_connectivity/plot_rest_parcellations.py as use this as the sole example for ReNA.

https://4066-1235740-gh.circle-artifacts.com/0/home/circleci/project/doc/_build/html/auto_examples/03_connectivity/plot_rest_parcellations.html#sphx-glr-auto-examples-03-connectivity-plot-rest-parcellations-py

[GaelVaroquaux]

I left a bunch of comments, but this is all I have. Thanks!

[GaelVaroquaux]

What do you suggest please, I am not sure to understand... To me, the threshold is simply a precision parameter,

You mean a tolerance? In which case it should be called "tol".

[kchawla-pi]

Let's call it `tolerance`.

…

On Thu, May 2, 2019, 19:12 Gael Varoquaux ***@***.***> wrote: > What do you suggest please, I am not sure to understand... To me, the threshold > is simply a precision parameter, You mean a tolerance? In which case it should be called "tol". — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1983 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6SXRG2GFOOSGEOGIKY4HTPTMOHFANCNFSM4HF5YASQ> .

[ja-che]

No to me, "tolerance" is the good nomenclature when optimizing something, here it is a precision parameter:

# Dealing with eccentricities, there are probably many neares neighbors
edge_mask = connectivity_.data > 1 - threshold

and

weights_ = np.maximum(threshold, weights_)

[kchawla-pi]

Then why not call it `precision` .

…

On Thu, May 2, 2019, 19:24 Jerome-Alexis Chevalier ***@***.***> wrote: No to me, "tolerance" is the good nomenclature when optimizing something, here it is a precision parameter: # Dealing with eccentricities, there are probably many neares neighbors edge_mask = connectivity_.data > 1 - threshold and weights_ = np.maximum(threshold, weights_) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1983 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6SXRFFUA4EGHXVQIFUO7DPTMPTPANCNFSM4HF5YASQ> .

[kchawla-pi]

There's a typo in neares of nearest neighbour .

…

On Thu, May 2, 2019, 19:25 Kshitij Chawla ***@***.***> wrote: Then why not call it `precision` . On Thu, May 2, 2019, 19:24 Jerome-Alexis Chevalier < ***@***.***> wrote: > No to me, "tolerance" is the good nomenclature when optimizing something, > here it is a precision parameter: > > # Dealing with eccentricities, there are probably many neares neighbors > edge_mask = connectivity_.data > 1 - threshold > > and > > weights_ = np.maximum(threshold, weights_) > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1983 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AB6SXRFFUA4EGHXVQIFUO7DPTMPTPANCNFSM4HF5YASQ> > . >

[GaelVaroquaux]

OK, I want this parameter removed from the parcellation object. It's an algorithmic detail. We make a choice as a good default, and remove it.

[kchawla-pi]

Cool. It's easy to add it back in if necessary. Removing it once we have added it is... well, I'll have to write another decorator. 😉

…

On Thu, May 2, 2019, 19:27 Gael Varoquaux ***@***.***> wrote: OK, I want this parameter removed from the parcellation object. It's an algorithmic detail. We make a choice as a good default, and remove it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1983 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB6SXREUUK475HT2OHHUQZDPTMP6PANCNFSM4HF5YASQ> .

[GaelVaroquaux]

Removing it once we have added it is... well, I'll have to write another decorator. 😉

How's your code? Decorated!

[ja-che]

@kchawla-pi
I don't get that one:

Warning, treated as error:
/home/circleci/project/nilearn/regions/parcellations.py:docstring of nilearn.regions.Parcellations:117:Unknown target name: "labels_img".

[ja-che]

@kchawla-pi
After rebuilding the error is different but still weird

[kchawla-pi]

I don't have a system at hand. Will look at it once I return, if it is still a problem.

[ja-che]

@GaelVaroquaux @bthirion @kchawla-pi
All lights are green, all comments are adressed, merge ?

[kchawla-pi]

@GaelVaroquaux I will give this one more glance but at this point it should be good to go. You have anything more to say?

[GaelVaroquaux]

Great! Congratulations!!