3
\$\begingroup\$

I am creating a class object which must inherit from numpy ndarrays. I perform an isinstance check and an array.shape check with the __new__ method. Is it alright to have it here, or should it be elsewhere? I saw suggestions to create an exceptions class to accompany it, but it doesn't seem necessary... maybe preferable.

'''
pnts_instantiation.py
'''
import numpy as np
import sys
from types import *

class Points(np.ndarray):
  '''ndarray required,info='array info',name='pnts'')
  '''
  def __new__(cls,arr=None,info=None,name='pnts'):
    '''create Points from existing data'''
    err = 'Failed...Requires an ndarray...\nProvided: {}'.format(type(arr))
    if isinstance(arr,(NoneType,StringType,UnicodeType,ListType,TupleType)):
      return err
    if arr.shape < (4,2):
      return ('Failed...Requires array shape > (4,2)\nProvided: {}'.format(arr.shape))
    self = np.asarray(arr).view(cls)   # view as Points class
    self.info = info          # set info
    self.name = name          # set name
    self.applied = None
    # other properties not pertinent to discussion removed for simplicity
    return self

  def __array_finalize__(new_arr, src_arr):
    '''new_arr: new Points object...housecleaning takes place
         for explicit, view casting or new from template...
       src_arr: None, any subclass of ndarray including our own OR another
         instance of our own array (see docs)'''
    if src_arr is None: return
    new_arr.applied = getattr(src_arr,'applied',None) # provide a default
    new_arr.name = getattr(src_arr,'name',None)

  def __array_wrap__(self,out_arr,context=None):
    '''wrap it up'''
    return np.ndarray.__array_wrap__(self, out_arr, context)

  def __repr__(self):
    '''return point info, shape and dtype'''
    s = self
    st = '==> {}'.format(s)
    if (hasattr(s,'name')) and (hasattr(s,'info')):
      st = 'name:  {}\nshape: {}\ninfo:  {}\nvalues:\n{}'.format(s.name,s.shape,s.info,s)
    elif (hasattr(s,'name')) and (hasattr(s, 'applied')):
      st = '{}.{}:  {}'.format(s.name,s.applied, s)
    else:
      st = '{}: {}'.format(s.applied,s)
    return st

def test_cases():
  '''conditional case check'''
  cases = [None,
           'string',
          [[1,2],[3,4]],
          np.asarray([[1,2],[3,4]],dtype='float64',),
          np.ma.asarray([[1,2],[3,4],[5,6]],dtype='float64'),
          np.asarray([[1,2],[3,4],[5,6]],dtype='float64'),
          np.asarray(zip(np.arange(5),np.arange(5)),dtype='float64')
          ]
  counter = 0
  for a_case in cases:
    print('\nCase: {}\nIn: {}\nOut: {}'.format(counter,a_case,Points(a_case)))
    counter += 1

#-------------------
if __name__ == '__main__':
  test_cases()
\$\endgroup\$
2
\$\begingroup\$

Well I agree with the exception sentiment. Errors are communicated by exceptions (unless it is an exceptional case, but really, what you have seems very standard, I don't see a reason to deviate here).

The typecheck for the input should probably be revised. It's not (immediately) clear to me what input arrays you actually want to accept. In any case the isinstance check should probably be done the other way round, i.e. check what type you want to accept (just np.ndarray?) instead of the ones you don't want (because the reader has to infer what other types there are). It might also be easier to defer the check until after np.asarray has returned a result, because afaik things like nested lists can actually be converted meaningfully as well (unless you don't want that obviously).

The shape check needs to be tighter as well, e.g. (4, 1, 1) < (4, 2) is still true.

The method __array_wrap__ is just passing through the values, so is it even needed?

The first argument of __array_finalize__ is self, so in the interest of clarity I'd use that as well, although I can see the point in expressing that it's the new array as well.

The use of hasattr in __repr__ strikes me as a bit odd, since in the construction of the object you do assign them, so is there a case when neither name, info, nor applied are available? I'd also add test cases if you rely on the output of repr.

As far as I can see from the subclassing doc the code seems fine otherwise.

Style-wise the indentation in Python code is four spaces, not two, there are spaces missing in some method argument lists and I'm sure a tool like pep8 will pick up more issues if you were to run it on the code.

\$\endgroup\$
  • \$\begingroup\$ I kept the isinstance in its current form, since there may be a time or place when I might want to include lists as well (still playing with masked arrays), but point taken. Good point on the shape check...hadn't thought of that. "array_wrap" was kept in solely to provide a marker in case I choose to override builtin/inherited methods, but point taken as with the finalize . The hasattribr syntax does indeed have cases where name and info will appear or name, info and applied. These are output formatting preferences so that I can keep track of them when various defs are used. \$\endgroup\$ – NaN Apr 9 '15 at 14:22
  • \$\begingroup\$ Forget some... In this way I can name the outputs so I can keep track of them onscreen whilst in interactive mode... Python code spacing not a requirement, I am in my 60's and now find it easier to read a page by looking at it when spacing is 4 (try to keep indentation down to a minimum as well)...so glad it is a PEP and not a requirement. I will reexamine pep8 in any event. \$\endgroup\$ – NaN Apr 9 '15 at 14:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.