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

Add importlib resources bases import to VisaInstruments #4790

Conversation

jenshnielsen
Copy link
Collaborator

@jenshnielsen jenshnielsen commented Nov 7, 2022

Cleaner implantation of #4789 which moves the logic into the visa instrument

TODO

  • Allow yaml files to be loaded from other modules
  • Docstrings and documentation (specifically writing simulated instrument notebook)
  • Tests?

@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Merging #4790 (239dbef) into master (d8f83ab) will increase coverage by 0.02%.
The diff coverage is 97.29%.

@@            Coverage Diff             @@
##           master    #4790      +/-   ##
==========================================
+ Coverage   68.20%   68.23%   +0.02%     
==========================================
  Files         339      339              
  Lines       32043    32065      +22     
==========================================
+ Hits        21856    21879      +23     
+ Misses      10187    10186       -1     

@jenshnielsen jenshnielsen force-pushed the use_importlib_resources_for_instrs_visa branch from a920db0 to 02523f5 Compare November 8, 2022 08:28
@jenshnielsen jenshnielsen marked this pull request as ready for review November 8, 2022 10:40
@jenshnielsen
Copy link
Collaborator Author

@astafan8 ready for review

Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

really nice long-due cleanup! thank you! (comments are very minimal, feel free to ignore)

qcodes/instrument/visa.py Outdated Show resolved Hide resolved
qcodes/tests/test_visa.py Show resolved Hide resolved
@jenshnielsen jenshnielsen merged commit c46b4d8 into microsoft:master Nov 8, 2022
@jenshnielsen jenshnielsen deleted the use_importlib_resources_for_instrs_visa branch November 8, 2022 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants