AFG3000 validators

[jakeogh]

I wanted to use AFG3000.py, but it relied on Multiples(divisor: float) which throws a TypeError:

 Traceback (most recent call last):                                                                                                                                                                                                                                               
  File "/usr/lib/python-exec/python3.9/qcodestool", line 12, in <module>                                                                                                                                                                                                         
    sys.exit(cli())                                                                                                                                                                                                                                                              
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1128, in __call__                                                                                                                                                                                                  
    return self.main(*args, **kwargs)                                                                                                                                                                                                                                            
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1053, in main                                                             
    rv = self.invoke(ctx)                                                                                                                                                                                                                                                        
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1659, in invoke                                                                                                                                                                                                    
    return _process_result(sub_ctx.command.invoke(sub_ctx))                                                                                                                                                                                                                      
  File "/usr/lib/python3.9/site-packages/click/core.py", line 1395, in invoke                                                                                                                                                                                                    
    return ctx.invoke(self.callback, **ctx.params)                                                                                      
  File "/usr/lib/python3.9/site-packages/click/core.py", line 754, in invoke                                                            
    return __callback(*args, **kwargs)                                                                                                                                                                                                                                           
  File "/usr/lib/python3.9/site-packages/click/decorators.py", line 26, in new_func                                                                                                                                                                                              
    return f(get_current_context(), *args, **kwargs)                                                                                                                                                                                                                             
  File "/usr/lib/python3.9/site-packages/qcodestool/qcodestool.py", line 119, in afg3022b                                               
    afg = afg3000.AFG3000(address='GPIB0::1::INSTR', name='afg3022b')                                                                   
  File "/usr/lib/python3.9/site-packages/qcodes/instrument/base.py", line 508, in __call__                                                                                                                                                                                       
    new_inst = super().__call__(*args, **kwargs)
  File "/usr/lib/python3.9/site-packages/qcodes_contrib_drivers/drivers/Tektronix/AFG3000.py", line 62, in __init__
    vals=vals.Multiples(divisor=0.1, min_value=0, max_value=120)
  File "/usr/lib/python3.9/site-packages/qcodes/utils/validators.py", line 492, in __init__
    raise TypeError('divisor must be a positive integer')
TypeError: divisor must be a positive integer

I ended up adding an AND version of MultiType: https://github.com/jakeogh/Qcodes/commit/8c79eb12ec7111f6882deb0c9fd51bb399d099fd

and patched AFG3000.py: https://github.com/jakeogh/Qcodes_contrib_drivers/commit/baa1c509c441d4863f77809aac8306df8d47d897

99% sure I'm missing something obvious. First time using Qcodes.

[jenshnielsen]

@jakeogh Thanks for your bug report. Your obervations looks correct to me. I would suggest that we first update the driver here to take a PermisiveMultiple validator. This will get the driver working but not catch all validation bugs

Then we can update the validator QCoDeS and then update the driver.

I would suggest that rather than creating a new validator we could extend the existing multiple validator.

E.g. something like def Multiple(*validators: Validators[Any], combiner: Literal['OR', 'AND'] = 'OR') and then extend the code to support both types of validation. Once that is in a release of qcodes we can update the driver again

Note that had this drivers init method been typed:

e.g. something like the following def __init__(self, name: str, address: str, **kwargs: Any): mypy would have caught this error so we could also add that

[jakeogh]

Hey, no problem. This software is a pleasure to work with.

Would it be reasonable to require the combiner? def MultiType(*validators: Validators[Any], combiner: Literal['OR', 'AND']), I made the AND case it's own class because I didn't want to introduce a way to unintentionally use a default value. If so, I would update all existing instances of MultiType.

Another thought is leave the MultiTypeAnd case as it's own class, with it's own tests, and optionally rename MultiType to MultiTypeOr (perhaps with alias and warning). I prefer this because it's explicit and adds some naming symmetry along with simpler docstrings and implementation.

Either way, I'm updating the existing MultiType to use the suggested function sig (which would require the least changes): def MultiType(*validators: Validators[Any], combiner: Literal['OR', 'AND'] = 'OR') and will post the patch.

[jenshnielsen]

Thanks for the update. How about that you update MultiParameter with the new signature and then create the and and or versions as subclasses of this. They can be really short since all they really need to do is override init to fix the value of the combiner

[jakeogh]

Ah, class MultiTypeOr(MultiType) and class MultiTypeAnd(MultiType)? If I understand that sounds perfect.

I might be misunderstanding, I see MultiParameter but I don't see the connection to this situation so I assume you meant MultiType.

WIP so far: https://github.com/jakeogh/Qcodes/commit/7e84dcb5f466154192e058082f6abb9f2756eb91

[jenshnielsen]

Sorry that was a typo yes indeed as you suggest 2 short subclasses cases for and and or