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

AFG3000: Use on_off_val_mapping instead of custom vals and get_parser #131

Merged
merged 1 commit into from
May 24, 2022

Conversation

mgunyho
Copy link
Contributor

@mgunyho mgunyho commented May 24, 2022

Before this change, setting boolean parameters to True/False failed silently:

from qcodes_contrib_drivers.drivers.Tektronix.AFG3000 import AFG3000

afg = AFG3000("AFG", address=...)

afg.state_output1.set(1)
assert afg.state_output1() is True  # ok

afg.state_output1.set(False)  # no error
assert afg.state_output1() is False  # fails, afg.state_output1 is still True !

# this works
afg.state_output1.set(0)
assert afg.state_output1() is False

With this patch, the second assert above also succeeds.

Note: while this is a bug in this driver, interestingly Enum(0, 1).validate(True) succeeds (works also with floats 0.0 and 1.0). Seems like this is due to hash(1) == hash(1.0) == hash(True) == 1. Perhaps this is a bit surprising, I would expect Enum to be a bit more strict. But maybe it's enough to warn about this in the documentation.

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2022

Codecov Report

Merging #131 (6cd28fd) into master (2fd9c39) will decrease coverage by 0.66%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
- Coverage   19.91%   19.24%   -0.67%     
==========================================
  Files         121      123       +2     
  Lines       14708    14629      -79     
==========================================
- Hits         2929     2816     -113     
- Misses      11779    11813      +34     
Impacted Files Coverage Δ
...codes_contrib_drivers/drivers/Tektronix/AFG3000.py 0.00% <0.00%> (ø)
qcodes_contrib_drivers/__init__.py 100.00% <0.00%> (ø)
...es_contrib_drivers/drivers/CopperMountain/M5180.py 0.00% <0.00%> (ø)
...contrib_drivers/drivers/NationalInstruments/DAQ.py 0.00% <0.00%> (ø)
...contrib_drivers/drivers/Tektronix/Keithley_6500.py 0.00% <0.00%> (ø)
...ontrib_drivers/drivers/NationalInstruments/RFSG.py 0.00% <0.00%> (ø)
...ontrib_drivers/tests/QDevil/test_sim_qdac2_init.py 100.00% <0.00%> (ø)
...rib_drivers/tests/QDevil/test_sim_qdac2_current.py 100.00% <0.00%> (ø)
..._contrib_drivers/drivers/GeneralMicrowave/GM349.py 0.00% <0.00%> (ø)
...codes_contrib_drivers/drivers/Holzworth/HS9008B.py 0.00% <0.00%> (ø)
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@astafan8
Copy link
Contributor

@mgunyho

Note: while this is a bug in this driver, interestingly Enum(0, 1).validate(True) succeeds (works also with floats 0.0 and 1.0). Seems like this is due to hash(1) == hash(1.0) == hash(True) == 1. Perhaps this is a bit surprising, I would expect Enum to be a bit more strict. But maybe it's enough to warn about this in the documentation.

indeed :) i haven;t looked at it, but it might be related to the fact that in python True == 1 etc, as you also mention. so we can just keep it that.

@astafan8 astafan8 enabled auto-merge May 24, 2022 10:38
Before this change, setting boolean parameters to True/False failed silently
@astafan8 astafan8 force-pushed the afg3000-bool-validators branch from 6cd28fd to a450330 Compare May 24, 2022 10:38
@astafan8 astafan8 merged commit fe59328 into QCoDeS:master May 24, 2022
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.

3 participants