feature: Add check whether instrument name is valid identifier

[maximilianluc]

Minor feature addition.
Checks whether instrument name is valid identifier. Whether name consists only of alphanumeric characters and underscores.

closes #4128

• Added check in InstrumentBase to verify if name of instrument/channel/module is valid identifier.
• Adjusted instrument name "AMI430-3D" -> "AMI430_3D" in relevant test files to adhere to valid identifier naming.

[codecov]

Codecov Report

Merging #4178 (8f569f9) into master (f8e77d7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4178      +/-   ##
==========================================
+ Coverage   68.38%   68.39%   +0.01%     
==========================================
  Files         232      232              
  Lines       30863    30872       +9     
==========================================
+ Hits        21105    21115      +10     
+ Misses       9758     9757       -1     

[jenshnielsen]

@maximilianluc Thanks for your pr.

A few technical details. We are using a formatter called darker which will basically run another formatter called black but only on the lines changed. To get the formatting job to pass you can do pip install darker[isort] and then run darker -r master qcodes

We have also set the repo up with a pre-commit hook this ensures that if you have it installed every time you commit you will run darker and other hooks automatically. You may want to set this up

[jenshnielsen]

To document changes we create a little file in docs/changes/newsfragments of the form pr number.type
In this case I think we should document this as a breaking change since some users may be creating instruments with invalid names. So in this case the file should be called 4178.breaking this file will later be merged with other changes to create a changelog

[maximilianluc]

Thanks @jenshnielsen, that'll help with the commits!

Perfect, will look into implementing it into the InstrumentBase class. Was thinking of it but wasn't sure if it was necessary for the others. I'll add a document to record the changes as well. Thanks!

[maximilianluc]

Moved the check to InstrumentBase class and added the docs changelog file @jenshnielsen!

Only having a bit of trouble with the pre-commit.ci - pr check which doesn't pass here (locally it does). Any clues?
Also, the pre-commit package didn't seem to work with miniconda because of a missing "_ctypes" source file. With the virtualenv package the pre-commit package does work smoothly.

[jenshnielsen]

@maximilianluc I think the bot is having some issues. I will give them a bit of time to recover and then come back to this

[jenshnielsen]

Looking at examples in qcodes contrib drivers I found a number of drivers that have a - in their names QCoDeS/Qcodes_contrib_drivers#134

I think it would be a good idea to rather than error on those replace the - with _ and print a warning if that is the only thing that makes the name invalid.

[jenshnielsen]

@maximilianluc We are hoping to do a new qcodes release within a week or so. Would you have time to implement the thing suggested above before that?

[maximilianluc]

Hi @jenshnielsen, yes for sure. Will try tomorrow, should be done before end of the week!

[maximilianluc]

Hi @jenshnielsen, it's done!

[jenshnielsen]

bors merge

[bors]

Build succeeded:

• builddocs (ubuntu-latest, 3.10)
• builddocs (ubuntu-latest, 3.9)
• builddocs (windows-latest, 3.7)
• pytestmin (ubuntu-latest, 3.7)
• pytestmypy (ubuntu-latest, 3.10)
• pytestmypy (ubuntu-latest, 3.7)
• pytestmypy (ubuntu-latest, 3.8)
• pytestmypy (ubuntu-latest, 3.9)
• pytestmypy (windows-latest, 3.10)
• pytestnonlocal (3.7)