better representation for parameter

[peendebak]

This gives a more informative output of a Parameter (and derived classes):

In [4]: param
Out[4]: <qcodes.instrument.parameter.StandardParameter: D1 at 140211932295464>

instead of

In [4]: param
Out[4]: <qcodes.instrument.parameter.StandardParameter at 140211932295464>

[AdriaanRol]

👍 💃

[giulioungaretti]

I'd avoid using string concatenation. I think this reads a bit better (and does not violate pep8):

class foo():

    def __init__(self, name):
        self.name = name

    def __repr__(self):
        s = '<{}.{}: {} at {}>'.format(
            self.__module__,
            self.__class__.__name__,
            self.name,
            id(self))
        return s

Maybe I sound silly, but a two liner test could also be a nice addition say somebody removes the name attribute, and does not update repr :D .

Eventually qcodes should have 100% code coverage. 🍷

[peendebak]

@giulioungaretti Using format is indeed nicer. I don't think adding a test on .name is a good idea, then we would have to do the same for every function in qcodes that used param.name.

[giulioungaretti]

@peendebak or @eendebakpt (I am confused :P ).
I agree not on .name but on repr() should be.
Just to cover the case, where name is None, string, empty, int.
Tiny detail, but the original implementation would have failed if self.name was not a string (and it could have been anything :D ). Having a test makes it easy to refactor afterwards !

If you need help to write the test just ping.

[peendebak]

@giulioungaretti I added explicit conversion to str for the .name field. That should catch quite some cases. I am not sure how to make a test for this. Do you mean something like

from qcodes import Parameter

def repr_test():
    param = Parameter(name='myparam')
    s=param.__repr__()

eendebakpt or peendebak is indeed confusing. I wish there was a credential manager for git that couples to the directories instead of the user login...

[giulioungaretti]

@peendebak not sure I understand what you say about git (but it would be nice to just have one of you contributing :P ).

Back to the code .format casts automagically the types, so the str() in 4beeffa are not required.
You can either git revert or git reset to get rid of that.
It was the string concat operator " + " (commit 3c771c1) that fails on non str-type.

The reason why I am stressing the test is that, if you log the class and there is something wrong with the repr, you will get an exception which is not really pleasant :D

Some pseudo code, assuming we have either a unittest.TestCase class or you add a method to a test class (f.ex in case of instrument.parameter, here )

Writing on my phone, so not really sure this is 100% correct 🌜 .

def repr_test(self):
    for i in [None, 0, "foo", "", "fåil"]:
            with self.subTest(i=i):
                param = Parameter(name=i)
                s = param.__repr__()
                #tweak this so it looks how you want the repr to look like 
                self.assertEqual(s , "foo.bar: {}  {}".format(i, id(param) )

[peendebak]

@giulioungaretti I do not see why the self.assertEqual in the test is useful. We are basically testing whether Parameter.__repr__()==Parameter.__repr__().

I added the test anyway, since if will not do much harm.

[giulioungaretti]

@peendebak yes it's a bit overkill for now, and yes assert equal is not required as anything that implements a str method will pass, my bad, owe you a beer!

But if one changes parameter class the test will now fail.
Making it easier to avoid bugs further down.

example:

class Parameter():

    def __init__(self, name):
        self.naame = name

    def __repr__(self):
        s = '<{}.{}: {} at {}>'.format(
            self.__module__,
            self.__class__.__name__,
            self.name,
            id(self))
        return s

or

example:

class Parameter():

    def __init__(self, name):
        self.name = name

    def __repr__(self):
        s = '<{}.{}: {} at {}>'.format(
            self.__module__,
            self.__class__.__name__,
            self._name,
            id(self))
        return s

[alexcjohnson]

@peendebak thanks, this is a good addition. @giulioungaretti thanks for pushing for robustness and tests. I suspect that we could make this DRYer as a mixin class (ReprWithName?) but hey, now that we have some tests we can do that safely any time we like 🎉

I changed some nonstandard indentation. I think we can 💃 !