get_parser not used in combination with val_mapping

[damazter]

If you encounter a bug use the following template.
If you have a feature request feel free to freestyle.

Steps to reproduce

1. make parameter with

        self.add_parameter('is_quenched',
                           get_cmd='QU?',
                           get_parser=str,
                           val_mapping={False: '0', True: '1'})

2. use instrument.is_quenched()

Expected behaviour

no error

Actual behaviour

I get the error (see below):
from which it is apparent it uses _valmapping_get_parser for which the docstring says:

"""
        Get parser to be used in the case that a val_mapping is defined
        and a separate get_parser is not defined.

        Tries to match against defined strings in the mapping dictionary. If
        there are no matches, we try to convert the val into an integer.
        """

So it seems to me that it ignores my get_parser

error traceback:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in _valmapping_get_parser(self, val)
    831             val = int(val)
--> 832             return self._get_mapping[val]
    833         except (ValueError, KeyError):

KeyError: 0

During handling of the above exception, another exception occurred:

KeyError                                  Traceback (most recent call last)
<ipython-input-9-e839900e5e48> in <module>()
----> 1 station.AMI430_3D._magnet_x.is_quenched()

d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in __call__(self, *args)
    127         if len(args) == 0:
    128             if self.has_get:
--> 129                 return self.get()
    130             else:
    131                 raise NotImplementedError('no get cmd found in' +

d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in get(self)
    809         except Exception as e:
    810             e.args = e.args + ('getting {}'.format(self.full_name),)
--> 811             raise e
    812 
    813     def _valmapping_get_parser(self, val):

d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in get(self)
    804     def get(self):
    805         try:
--> 806             value = self._get()
    807             self._save_val(value)
    808             return value

d:\pycharmprojects\qcodes\qcodes\utils\command.py in __call__(self, *args)
    175             raise TypeError(
    176                 'command takes exactly {} args'.format(self.arg_count))
--> 177         return self.exec_function(*args)

d:\pycharmprojects\qcodes\qcodes\utils\command.py in call_by_str_parsed_out(self, *args)
    128     def call_by_str_parsed_out(self, *args):
    129         """Execute a formatted string with output parsing."""
--> 130         return self.output_parser(self.exec_str(self.cmd_str.format(*args)))
    131 
    132     def call_by_str_parsed_in(self, arg):

d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in _valmapping_with_preparser(self, val)
    835 
    836     def _valmapping_with_preparser(self, val):
--> 837         return self._valmapping_get_parser(self._get_preparser(val))
    838 
    839     def _set_get(self, get_cmd, get_parser):

d:\pycharmprojects\qcodes\qcodes\instrument\parameter.py in _valmapping_get_parser(self, val)
    832             return self._get_mapping[val]
    833         except (ValueError, KeyError):
--> 834             raise KeyError('Unmapped value from instrument: {!r}'.format(val))
    835 
    836     def _valmapping_with_preparser(self, val):

KeyError: ('Unmapped value from instrument: 0', 'getting AMI430_X_is_quenched')

System

operating system
win7
qcodes branch
local, related to driver/AMI_430
qcodes commit
latest merge with master
de88c36

[giulioungaretti]

@damazter quick comment of which I am not sure about: isn't the val mapping supposed to be the other way around ? {'0':False,'1':True }

[Rubenknex]

@giulioungaretti I think it's right how it is now.

@damazter For some reason it acts as if the get_parser is None and then skips it

[damazter]

@Rubenknex
It is also running through _valmapping_with_preparser so maybe I am fundamentally misunderstanding how this part of parameter works

[giulioungaretti]

@alexcjohnson can explain you 🦄

[damazter]

@giulioungaretti
from the docstring

"""
        val_mapping (Optional[dict]): a bidirectional map data/readable values
            to instrument codes, expressed as a dict:
            ``{data_val: instrument_code}``
            For example, if the instrument uses '0' to mean 1V and '1' to mean
            10V, set val_mapping={1: '0', 10: '1'} and on the user side you
            only see 1 and 10, never the coded '0' and '1'
"""

[alexcjohnson]

also from the docs:

You can use ``val_mapping`` with ``get_parser``, in which case
``get_parser`` acts on the return value from the instrument first,
then ``val_mapping`` is applied (in reverse).

You CANNOT use ``val_mapping`` and ``set_parser`` together - that
would just provide too many ways to do the same thing.

So yeah, it should work. Seems like for whatever reason the value it's getting is something that will cast to 0 but isn't '0'... perhaps it's getting '0\n' or something? If that's it, you can either:

• use integers in the val_mapping, ie: val_mapping={False: 0, True: 1} and take advantage of the implicit casting to integer.
• fix the string version - perhaps the instrument would benefit from defining a terminator?

To make it easier to debug in the future, we should keep track of the original value passed into _valmapping_get_parser and report that instead of after attempting to cast it to integer.

Anyway get_parser=str, while I don't think it's the problem here, isn't going to do anything if you already have a string that you read from the instrument.

[giulioungaretti]

@alexcjohnson thanks!

[damazter]

@giulioungaretti
@Rubenknex
@alexcjohnson

Comparing integers fixes all my problems, I made a pr to the relevant driver. For me this closes the issue, but if you want to solve the underlying problem as well 'giulio' you can keep this open, otherwise, just close it

[alexcjohnson]

Ah I see, your AMI430 driver is an IPInstrument - and it looks like we don't strip terminators on read for IPInstruments, even though you have one defined. Perhaps another thing we should fix...

[giulioungaretti]

@damazter I think I will close this. The get_parser is indeed working as expected!

For posterity sake:
in this case the instrument was returning something like:

 "0\n" 

which is not equal to "0"

but casting to int:

int("0\n") 

which is equal to 0