Skip to content
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.

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

AMI430 3D driver: allow instantiating from names of existing AMI430 instances, next to from AMI430 instances only #2949

Merged
merged 2 commits into from
Apr 22, 2021

Conversation

astafan8
Copy link
Contributor

@astafan8 astafan8 commented Apr 21, 2021

Useful when defining AMI430_3D in station YAML config file (after defining the individual AMI430 axes)

as requested by @jdwatson.

also: Make error message explicit when instrument with name does not exist.

@astafan8 astafan8 added this to the 0.25.0 milestone Apr 21, 2021
@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #2949 (f818bf6) into master (9fb4244) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2949   +/-   ##
=======================================
  Coverage   65.41%   65.42%           
=======================================
  Files         209      209           
  Lines       28122    28127    +5     
=======================================
+ Hits        18397    18403    +6     
+ Misses       9725     9724    -1     

@astafan8 astafan8 enabled auto-merge April 21, 2021 18:35
Copy link
Collaborator

@jenshnielsen jenshnielsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Left a few questions inline. Is this worth documenting in the example notebook ?

qcodes/instrument/base.py Show resolved Hide resolved
mag_x.name, mag_y, badly_typed_instrument_z_argument,
field_limit
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add a test for a badly named instrument. E.g. a string but not the name of an AMI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i'll add a variation of the test_instantiation_from_name_of_nonexistent_instrument for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants