Suggestion of replacing ```theParams``` (a class) with the ```**kwargs```(a dictionary) for the optional arguments of the class constructor

@pv33 @Uio96

I noticed that our current code translation creates an additional parameter class for the optional arguments of the class constructor with the attempt to imitate the usage of the Matlab version. An example is: https://github.com/ivapylibs/perceiver/blob/6a18158d1914d3ed4f019d62ee669063b1e93ca4/perceiver/simple.py#L76

May I suggest to replace that with the Python's own **kwargs (keyword arguments? I believe the biggest advantage is that the kwargs will be more flexible, as you don't need to create a class instance for passing the arguments, which will make the code complicate if the class number we used is increasing:

# when need multiple classes
import classA, classB, classC

# if using class-style parameters, assuming the class requires no positional arguments and all arguments are optional
import classA_params, classB_params, classC_params
paramA = classA_params(a=1,b=2); paramB = classB_params(c=1,d=2); paramC = classC_params(e=1,f=2); 
A = classA_params(paramA); # same for B and C

# if using kwargs
A = classA(a=1, b=2) # same for B and C

Also if you want to add another argument, the class-style requires modifying the class definition, whereas the kwargs allows simply appending the new arguments to the argument list:

# add another optional argument "c" for the classA above
A = classA(a=1, b=2, c=3)

I understand we are trying to follow the convention of the Matlab, but I think the python convention is a neater way to do the job. I know it is better to be consistent about the naming across all libraries, so I think I need to bring it up for discussion.

I put the issue here because I need the positional arguments for passing some pre-processing or post-processing function when coding up the layer splitter. And I think we don't have to change everything we have already created, but I do think it is more convenient for me to use kwargs in this repository

@yiyeChen @Uio96 Most likely this implementation is superseded by dataclass or some other modern python3 affordance. I recommend fully reviewing the options before moving towards the use of kwargs.

link 1 link 2 Stack overflow discussion. see second or third answer. It also suggests named tuples. In both cases, the instantiation approach you prefer with strings applies. MRO inheritance applies. I didn't fully grok all of your arguments or justifications, but I suspect that dataclass and named tuples pretty much do it all. My preference is for dataclass. As long as we are all using a modern python3 implementation, we should not be concerned with back-portability.

kwargs is going fully to the limit in another direction in a way that might not be best. The above seems like a pleasant middle ground.

Generally speaking, the concept of such a parameter structure should not be unique to Matlab. I believe I have seen similar things in other APIs for C++ or maybe even C (in particular APIs for implementing optimization or optimal control schemes). Maybe even in python. Thus, if some alternative will be argued for, first see what good coding practice has been done already in python to reproduce the same parameters affordance. Not at the base or syntactic python level, but at the level of APIs and code packages that involve the same functionality.

One beauty of dataclass is the specification of default values. Doing so eliminates the need for setIfMissing, if I am not mistaken. Would need to verify that.

Now regarding your needs:

I put the issue here because I need the positional arguments for passing some pre-processing or post-processing function when coding up the layer splitter. And I think we don't have to change everything we have already created, but I do think it is more convenient for me to use kwargs in this repository

I don't fully understand what you want to do, nor how that triggers this need. Please post an issue for that specific problem somewhere and explain it fully. My sense is that you are violating the spirit of the implementation and most likely should be doing something else.

@pv33 @Uio96

I will just post there. I think there is nothing kwargs can do that the class can't. I just think if when they function the same, kwargs is more convenient.

For example, what triggers my need is that I am creating I am creating a multi-level nested classes, such as:

class simple() # from the perceiver repo
class Base(simple)
class Base_fg(Base)
class Hand_seg(Base_fg)

And each level of them will require their own unique optional arguments. In this case, when using kwargs you can easily pass the kwargs down to the root classes, such as (I am ignoring the positional arguments):

class simple() :
    def __init__(self, **kwargs):
        if kwargs['display']:
             # do something

class Base(simple): 
    def __init__(self, **kwargs):
        super().__init__(**kwargs) # easily pass the kwargs to the root
        if kwargs['preprocess']:
             # do something

If you use a class or a dataclass, you can also make it work, but in a little more complicated way, since you will need to create a new data class for each child class you created, and that data class has to inherit the parent class's data class, something like this:

class param_simple():
    def __init__(self, display=None):
         self.display=display

class param_Base(param_simple):
    def __init__(self, display=None, preprocess=None):
         super().__init__(display)
         self.preprocess = preprocess

Moreover, when you want to add another parameter during the development, you will need to modify the data class first.

But like I said, I know it can work, I just see no point of making it complicated if it can not offer more features, and the kwargs in my opinion is more flexible, in the sense that you can pass anything in, and inside your class definition you can define what you want to use.

Also I searched for coding practice as you suggested. I found that lots of them choose to simply lay all the arguments out in the list instead of packing them together, like the pytorch dataloader or the matplotlib.figure. But in function definitions kwargs are use alot, like the matplotlib.pyplot.plot, in which kwargs are used pack a bunch of optional plotting arguments like alpha, linewidth, etc.

I admit I am failing to see the difference between those approaches from the functionality perspective. Do you have any ideas on the pros and cons other than the convenience?

@yiyeChen Please go read up on dataclass. Your interpretation of them seems to be off. They have inheritance. Please confirm the below checklist item:

[x] Yiye has read up on the python 3.7 dataclass addition and fully understands the affordances of the dataclass construction.
[x] Yiye actually sees that even on the fly/dynamic fabrication through lambda-like code is possible, with inheritance of a base dataclass. link
[x] Yiye understands that the entire point of the dataclass construct is to automatically generate all of the silly constructor code and set/get routines, and lots of other elements. It also handles inheritance in an MRO fashion (derived class can override base class fields). Construction and use is simply a matter of defining the class and the fields. Defining the fields provides for a mechanism to document their use, thereby killing two birds with one stone.
[x] Yiye understands that it provides for default values, thereby preventing the need for setIfMissing types of checks.

I am not convinced at all by your argument(s). I don't see the problem with defining parameter subclasses within each classes scope, as in:

Base.paramSpec
Base_fg.paramSpec
Base_Hand_seg.paramSpec

based on

@ dataclass
class paramSpec:
  ETC

in Base_fg scope:
@ dataclass
class paramSpec (Base.paramSpec):
  ETC

In Base_Hand_seg scope:
@ dataclass
class paramSpec(Bass_fg.paramSpec):
  ETC

I am quite averse to destroying the namespace/scoping thus the fact that they have the same class name at the end of the namespace chain is not a problem to me. I see it as advantageous. It derives from "the coding is the documentation" spirit of things.

Having to specify the class is a form of documentation and only costs about 1-2 minute of coding and documentation time. To want to save 1-2 minutes of coding and documentation time per class doesn't seem like a strong argument.

Also, if they are not different then the best is to stop arguing for them. Just start using dataclass and move on.

The argument that kwargs is more flexible is lost on me. Please provide concrete evidence of what it does that dataclass cannot do and how mission critical it is to your task. Minor evidence of convenience is not an excuse. Sometimes I see convenience as a mark of the lazy. This is one such instance because the justification is about small effort savings.

Now, what you may be doing is adding random parameters that conditionally trigger code based on their existence or not. Doing so seems dangerous to me. Not sure that programming in that style is good because then there are hidden parameters (possibly undocumented) that affect functionality.

@pv33

Sorry I didn't read carefully. I now see how how dataclass decorator adds upon the class the desirable features.

Also I didn't think thoroughly. I agree now that the "coding to document" style can make things clearer.

I will close that issue and modify the code accordingly.

ivapylibs / Surveillance

Suggestion of replacing ```theParams``` (a class) with the ```**kwargs```(a dictionary) for the optional arguments of the class constructor #5