[PATCH] Fix issues in Points.__init__ method

2013-05-31
2013-11-04
  • Hi,
    Reading the code of Point __init__ method I’ve found several inconsistencies and
    issues. Some of them:

    • When Point is build from ‘coordict’:
       • it’s possible to provide coord value as a list, later this list is
         silently truncated to the first element only, without type checking the value
       • it’s possible to provide coord value as a array, later this array is
         silently truncated to the first element only. In the case when length of
         array is equal to 1, __init__ fails with mysterious IndexError. So this
         code snippet works:
        

             Point({'x': [0.0]})
    

          but this doesn’t
        

             Point({'x': array([0.0])})
    

       • this check
        

              assert not isinstance(coorddict[c], (list, tuple)), 'type mismatch'
    

         is useless as all possible values in coorddict are converted to array
         during coorddict building

       • if ‘coordtype’ argument is not provided, then self.coordtype is set to
               ‘float’ and is not equal self.coordarray.dtype.type (which is float64)

    • self.coordarray and self.coordnames are built in different ways from
    ‘coorddict’ and ‘coordarray’/’coordnames’ pair but nevertheless there a lot of
    duplicates in code (for example, type conversion and rank checking)

    I’ve made the testsuite for Point and refactored a bit its __init__ method.
    New version of Point __init__ method:
    • uses the only way for building Point attributes
    • warns user when provided list/array value for coord is truncated
    • uses 'float' as default value for coordtype. To set it to ‘int’ user should
    explicitly set ‘coordtype’ argument or pass array with int values
    • has, I hope, cleaner and simpler code with less nested structure

    I've submitted the patch on SourceForge:
    https://sourceforge.net/tracker/?func=detail&aid=3614176&group_id=140858&atid=747667

    Any feedback is appreciated.

    I use 'nose' to run tests:

    cd tests && nosetests point.py
    

    Try it before patching Points.py to see failures fixed by patch.

    PS. I would like to get involved in PyDSTool development. Let me know if you are interested in it.

    Regards,
    Vladimir Zakharov

     
  • Rob Clewley
    Rob Clewley
    2013-05-31

    Thanks for your observations, they are helpful. I will look over your patch soon and get back to you. I am interested in dev help, so please email me privately (e.g. via the SF project's mail link or my home web site) and we can discuss it further.

    -Rob