Managing user defined and default values for instance attributes

Question

The following works as expected but I wonder if there is a more idiomatic way to check the input kwargs against the user-required (non-defaulted) arguments.

It is written in this way: so that as I continue to develop and modify my script and add attributes, I simply need to add them to the class variable defaults = {'A':None, 'B':0, 'C':0} and set it to None if the user is required to specify it. I also like that managing this as a class variable is visible at this time.

I've included a modified adaptation from item #6 in this excellent answer that makes sure all of the arguments end up as either floats or np.arrays with all the same length.

• If they are: the attributes are set to default or user values and .ok set to True

• If not: attributes are not set to defaults or user values and .ok remains False

In this example, a value for A is required of the user. They may enter values for B and C but if not those are initialized to 0.0. Any extra arguments such as D=42 will just be ignored.

import numpy as np
class O(object):
 defaults = {'A':None, 'B':0, 'C':0}
 required = [key for (key, value) in defaults.items() if value == None]
 ok = False
 def __init__(self, **kwargs):
 if not all([key in kwargs for key in self.required]):
 print('problem, something required is missing')
 setup = self.defaults.copy()
 for (key, value) in kwargs.items():
 if key in setup:
 setup[key] = kwargs[key] # user specified overrides default
 setup = self.fixem(setup)
 if setup:
 for (key, value) in setup.items():
 setattr(self, key, value)
 self.ok = True
 else:
 print('something did not work')
 def fixem(self, setup):
 # adapted from https://codereview.stackexchange.com/a/233169/145009
 results = None
 keys, values = zip(*setup.items())
 arrays = list(map(np.atleast_1d, values))
 sizes_ok = len(set(map(np.size, arrays)).difference(set((1,)))) <= 1
 all_1d = set(map(np.ndim, arrays)) == set((1,))
 all_good_types = all(array.dtype in (np.int64, np.float64) for array in arrays)
 if all([sizes_ok, all_1d, all_good_types]):
 arrays = [array.astype(float) for array in arrays] # make all arrays np.float64
 values = list(map(lambda x: float(x) if len(x) == 1 else x, arrays)) # downcast length=1 arrays to float 
 results = dict(zip(keys, values))
 return results 
# TESTING:
attrs = ('A', 'B', 'C')
print('\nBEGIN good seup testing: ')
o = O(A=42)
print("\nEXPECT:[('A', 42.0), ('B', 0.0), ('C', 0.0)]")
print('GOT: ', [(attr, getattr(o, attr)) for attr in attrs if hasattr(o, attr)])
o = O(A=[1, 2, 3], B=np.exp(1), C=np.array([2, 3, 4]))
print("\nEXPECT:[('A'. array([1., 2., 3.])), ('B', 2.718281828459045), ('C', array([2., 3., 4.]))]")
print('GOT: ', [(attr, getattr(o, attr)) for attr in attrs if hasattr(o, attr)])
print('\nBEGIN bad seup testing: \n')
o = O(B=42)
print('\nEXPECT:[] (i.e. nothing!)')
print('GOT: ', [(attr, getattr(o, attr)) for attr in attrs if hasattr(o, attr)])
o = O(A=[1, 2, 3], B=[1, 2, 3, 4])
print('\nEXPECT:[] (i.e. nothing!)')
print('GOT: ', [(attr, getattr(o, attr)) for attr in attrs if hasattr(o, attr)])

OUTPUT:

BEGIN good seup testing:
EXPECT:[('A', 42.0), ('B', 0.0), ('C', 0.0)]
GOT: [('A', 42.0), ('B', 0.0), ('C', 0.0)]
EXPECT:[('A'. array([1., 2., 3.])), ('B', 2.718281828459045), ('C', array([2., 3., 4.]))]
GOT: [('A', array([1., 2., 3.])), ('B', 2.718281828459045), ('C', array([2., 3., 4.]))]
BEGIN bad seup testing:
problem, something required is missing
something did not work
EXPECT:[] (i.e. nothing!)
GOT: []
something did not work
EXPECT:[] (i.e. nothing!)
GOT: []

• camp0: In the constructor you may want to through exceptions such as "ValueError" instead your print statements for example. In general I dont like to have classes that can process different parameters depending on types and so on, this normally brings me the idea that my class is not correctly defined.
• uhoh: @camp0 Yes indeed, the print statements are (unattractive) placeholders. Once I finalize the the overall way this is going to work I'll treat exceptions in a systematic way.

Answer 1

Some minor comments on the code:

1. Class definition should be separated from the line with import by two spaces.

2. My personal preference is to have key-value pairs in dictionaries separated with a space after colon as shown in PEP 8:

   defaults = {'A': None, 'B': 0, 'C': 0}
   

3. Comparing to None should be done by is instead of ==:

   required = [key for key, value in defaults.items() if value is None]
   

   Note that I also removed redundant brackets around key, value. There are several other lines where brackets are not needed around them.

4. PEP 8 also discourages aligning several lines with assignments by =, so instead of, for example:

   results = None
   keys, values = zip(*setup.items())
   

   it should be

   results = None
   keys, values = zip(*setup.items())
   

5. There is no need to specify object in class O(object), class O will work fine.

6. Here:

   for key, value in kwargs.items():
    if key in setup:
    setup[key] = kwargs[key] # user specified overrides default
   

   you don't use value, but you could:

   for key, value in kwargs.items():
    if key in setup:
    setup[key] = value
   

7. Here:

   keys, values = zip(*setup.items())

   you don't need values as you overwrite them later. So, I'd just remove this line altogether.

8. set((1,)) can be replaced with {1}, and set.difference can be replaced with just -. BTW, I like how you combined two conditions from my previous review in one!

9. Don't forget to use np.can_cast instead of checking the dtypes against np.int64. The current version failed for me until I changed it.

10. [array.astype(float) for array in arrays] can be written as list(map(np.float64, arrays)) but both versions are fine.

11. The overall design looks quite unusual to me. If it would be me, I'd separate validating data from the container that will keep it. In other words, I'd not keep it in one class. BTW, if a class has just two methods and one of them is __init__ then it shouldn't be a class. Another thing you could try is pydantic library. Never had a chance to try it myself though, but with this problem of data validation I'd give it a shot.

• uhoh: Once again, thank you for the review and tutelage! There's a lot of habits to break here (some badder than others) but I'll work on all of them.
• uhoh: I'm now slowly de-classing; this answer keeps on giving!

Answer 2

If there are required parameters, you should state them explicitly.

class O:
 def __init__(self, A=None, B=0, C=0, **kwargs):

By all means I would advise stronly against your solution. Class members should be clearly readable.

• uhoh: I know this is the standard way, but in what way is having the next line with the class variable defaults = {'A':None, 'B':0, 'C':0} unreadable in comparison? I'll need to check again why I wanted defaults available as a class variable. If I can't identify a clear need for that then this is fine; if there is a need for it I'll update. Thanks!
• Piotr Rarus: You're dynamically adding members, which are not dynamic. What for? Intellisense won't help you in any way later.
• Piotr Rarus: You can access parent members from subclass, so there would be no need to reimplement this design in subclasses. That's what inheritance is for.
• Piotr Rarus: Additionally, please run some linter, before you post here. It'll make bitches like me much happier. You can also use auto-formatter like black.
• Piotr Rarus: Python creators and community are really caring, comparing to other, like .net and c#. Say your prayer everyday. Be familiar with style guides PEP8. Linters are tools which help you ensure compatibility with PEP8. There's plenty of them i.e. flake8, pylint. If you're using vscode or pycharm it'll automatically detect those linters in your env and highlight errors.