agramfort/scikit-learn — comment on commit 367cd4ad150f493a4f95ccc87a86d9cabf1868c5
File:  line 0
By jmetzen:

Hi, sorry for the late response, I was quite busy yesterday and did not find the time to look at your code in detail.

Some comments:
a) For implementing a new linkage criterion, one has to implement in your design a setup function (like _ward_setup) and a callable (like _Ward). This corresponds to the __init__ method (for setting things up) and the update method (corresponding to the callable). I can go with your design; I am just curious why you guys prefer this approach?

b) What dou you think about renaming the variables "A", "coord_row", and "coord_col"? I don't think the names are very intuitive. I have also the impression that there is some redundancy between A and "coord_row"/"coord_col". Maybe there would be an other data structure that could unify these three variables. Have to think about it...

c) Your design: data representation by named tuple, several module level function that act on it. My design: data representation in class and methods that provide functionality. I'm personally more inclined to the object-oriented design (as you might have guessed), but we can also go with yours. Just for understanding: Why do you prefer it? Just for efficiency? Is there really such a big difference?