[pymvpa] dissimilarity stuff

Yaroslav Halchenko debian at onerussian.com
Fri Apr 24 19:09:44 UTC 2009


Hi James,

once again thank you for contribution.

I've just now pushed 2 commits which touched your code:

1st one 
0c962d2b83b4fd8f4c4b45d0a8dfaa3385aa730d
is plain reformatting a bit -- please go through it to see what
I've done

2nd one 
957b7cca4c9782a52aff241ff9a64b6070b18405
is more intrusive: I had to strip DSMDatasetMeasure away from
base.py since it doesn't belong there: it is a particular implementation
dependent on scipy (due to DSMatrix itself dependency) -- so I placed it
under measures/ds.py

Unless I do so -- unittest-badexternals breaks since mvpa.measures.base
would fail to import since there would be no scipy and that should
not happen -- scipy is an optional dependency.

Please go through those 2 commits -- I also left some comments/questions
marked with XXX.

Also unittests and examples are still due I guess? ;) without them I
could not even verify that my changes do not break your code... which is
not a good thing of cause

On Mon, 06 Apr 2009, Michael Hanke wrote:

> HI James,

> On Sat, Apr 04, 2009 at 01:30:14PM -0400, James M. Hughes wrote:
> > Hi all,

> > I just pushed into my branch an implementation of some stuff from  
> > Kriegeskorte's representational similarity analysis paper.  Well,  
> > actually it's a generic (unoptimized) implementation of a  
> > dissilimarity matrix, along with a "DSMDatasetMeasure" (i.e.,  
> > dissimilarity dataset measure) for using dissimilarity matrices in  
> > searchlight algorithms, etc.

> > The code's not pretty and might break things, so please have at it!

> First of all: Thanks for your contribution! I know it is much easier to
> keep code on a private harddisk instead of risking public review -- I
> very much appreciate that you have decided to take this path ;-)

> So far I have just glanced over the code and would like to ask you for a
> few modifications:

> 1. There is neither an example, nor any unittest for the new
>    functionality. If I, or anyone else wants to start polishing your
>    code they would be forced to craft a unittest first to ensure that
>    any refactoring does not change the intended behavior. However, it is
>    much more efficient if the original author provides these tests.

>    If you want to attract users to test your code in their analysis the
>    typically benefit from a simple example. If possible just make use of
>    the example dataset that we ship inside the pymvpa source tarball.
>    Ideally the example would generate a figure that is similar to what
>    people would recognize from the literature. If you need other/more
>    data -- just tell me.

>    Both things should be easily doable since most likely you already
>    have some test code that just needs to be turned into a proper
>    unittest, if you need help with that just push the code.

> 2. Documentation. You have lots of valueable information in comments
>    (e.g. type of input arguments, notes on behavior, ...). In the
>    comments it is hardly accessible by users. Please move that
>    information inside the docstrings.


> Looking forward to see how it goes!


> Cheers,

> Michael
-- 
Yaroslav Halchenko
Research Assistant, Psychology Department, Rutgers-Newark
Student  Ph.D. @ CS Dept. NJIT
Office: (973) 353-1412 | FWD: 82823 | Fax: (973) 353-1171
        101 Warren Str, Smith Hall, Rm 4-105, Newark NJ 07102
WWW:     http://www.linkedin.com/in/yarik        



More information about the Pkg-ExpPsy-PyMVPA mailing list