New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make parameter registration on instrument part of parameter init #3191

Merged

jenshnielsen merged 36 commits into microsoft:master from jenshnielsen:make_parameters_attrs_on_instr

Jul 22, 2021

Collaborator

jenshnielsen commented Jul 9, 2021

Such that parameters can be regular attributes on the instrument. This is lifted from #3188

codecov bot commented Jul 9, 2021 •

edited

Codecov Report

Merging #3191 (653aa79) into master (61319ec) will increase coverage by 0.02%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master    #3191      +/-   ##
==========================================
+ Coverage   65.92%   65.95%   +0.02%     
==========================================
  Files         218      218              
  Lines       28943    28966      +23     
==========================================
+ Hits        19082    19104      +22     
- Misses       9861     9862       +1

astafan8 reviewed

View reviewed changes

Contributor

astafan8 left a comment

Don't we want to advertize in the notebook that this way ofdeclaring parameters is now possible?) or is that left for further PRs?

qcodes/instrument/parameter.py Outdated Show resolved Hide resolved

qcodes/instrument/parameter.py Show resolved Hide resolved

qcodes/instrument/parameter.py Outdated Show resolved Hide resolved

Collaborator Author

jenshnielsen commented Jul 9, 2021

Don't we want to advertize in the notebook that this way ofdeclaring parameters is now possible?) or is that left for further PRs?

Yes it needs some docs work before merging. I broke this out so that we could confirm individually that this is sound (the other pr breaks tests due to non related reasons)

jenshnielsen commented

View reviewed changes

qcodes/instrument/parameter.py Outdated Show resolved Hide resolved

jenshnielsen commented

View reviewed changes

qcodes/instrument/parameter.py Outdated Show resolved Hide resolved

jenshnielsen commented

View reviewed changes

qcodes/instrument/base.py Outdated Show resolved Hide resolved

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch 4 times, most recently from be5a378 to 7803335 Compare

July 13, 2021 11:54

jenshnielsen commented

View reviewed changes

qcodes/instrument/base.py Show resolved Hide resolved

jenshnielsen commented

View reviewed changes

qcodes/instrument/parameter.py Show resolved Hide resolved

jenshnielsen commented

View reviewed changes

qcodes/tests/instrument_mocks.py Outdated Show resolved Hide resolved

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch from 7803335 to 9500c94 Compare

July 13, 2021 12:49

jenshnielsen commented

View reviewed changes

qcodes/instrument/delegate/delegate_instrument.py Outdated Show resolved Hide resolved

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch from 9500c94 to c13137c Compare

July 13, 2021 13:08

jenshnielsen commented

View reviewed changes

docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb Show resolved Hide resolved

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch from a74fe4c to 351512f Compare

July 14, 2021 09:55

jenshnielsen changed the title ~~Make parameter registration part of parameter init~~ Make parameter registration on instrument part of parameter init

astafan8 added this to the 0.28.0 milestone

jenshnielsen mentioned this pull request

forward instrument to parameter baseclass in rto driver #3205

Merged

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch 2 times, most recently from ab55c75 to 8a2a18b Compare

July 19, 2021 11:32

jenshnielsen mentioned this pull request

A new way of adding parameters that allows overriding inherited parameters (and sphinx auto documentation) #3098

Open

4 tasks

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch from e57e32e to cdfc0ff Compare

July 20, 2021 11:17

astafan8 reviewed

View reviewed changes

qcodes/instrument/base.py Show resolved Hide resolved

qcodes/instrument/parameter.py Outdated Show resolved Hide resolved

qcodes/tests/parameter/test_parameter_registration.py Outdated Show resolved Hide resolved

jenshnielsen added 4 commits

July 21, 2021 09:31


          move parameter registration logic to param class

bbea2a4


          make binding to instrument optional and forward kwargs from array and…

c4a2265

… multiparam


          preserve error api

32af381


          dont register parameter twice

6ec8078

jenshnielsen and others added 22 commits

July 21, 2021 09:31


          Add basic test for param registration

8a381fc


          add_parameter register on instr with warning

334c085


          test that parameter is added but warns when broken

5f33e41


          add docstrings

4cb06d6


          Remove fstring


          fix line lenght

dcc8f48


          convert part of notebook to new style

43a0ffd


          dont bind delegate parameters by default but do bind by default from …

03caeb3

…add_parameter


          add missing type

c397870


          keep api compatible

321eae6


          bind parameters in delegate instr

7b24a22


          forward kwargs to param base class

2dde2ca


          missed one

f54de07


          fix docstring

218ad61


          test warning is raised

aaad6e9


          Add note about delegateparam registration

3ad6c7c


          update notebook

d8d1500


          handle deprecation in delegate instrument too

b94d2fe


          Avoid code duplication by reusing add_parameter

62b054c


          Update qcodes/tests/parameter/test_parameter_registration.py

5f5c9c8

Co-authored-by: Mikhail Astafev <astafan8@gmail.com>


          move to end to avoid registering parameters that throw

6cc31ef


          rerun darker

fa071da

jenshnielsen force-pushed the make_parameters_attrs_on_instr branch from 29be8f9 to fa071da Compare

July 21, 2021 07:31

astafan8 approved these changes

View reviewed changes

docs/examples/writing_drivers/Creating-Instrument-Drivers.ipynb Outdated Show resolved Hide resolved

qcodes/instrument/base.py Outdated Show resolved Hide resolved

qcodes/instrument/base.py Outdated Show resolved Hide resolved

qcodes/instrument/base.py Outdated Show resolved Hide resolved

jenshnielsen and others added 2 commits

July 22, 2021 16:35


          Apply suggestions from code review

2848da0

Co-authored-by: Mikhail Astafev <astafan8@gmail.com>


          fix typo in notebook

jenshnielsen enabled auto-merge

July 22, 2021 14:39


          adapt test to warning change

653aa79

jenshnielsen merged commit c66a8af into microsoft:master

jenshnielsen deleted the make_parameters_attrs_on_instr branch

July 22, 2021 15:35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment