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

Fix typechecking with newest qcodes and Numpy #116

Merged
merged 6 commits into from Feb 10, 2022

Conversation

jenshnielsen
Copy link
Collaborator

@jenshnielsen jenshnielsen commented Feb 10, 2022

With newer versions of qcodes the typing of ChannelList has improved to reflect that __getitem__ always returns a InstrumentChannel when called with an int so this cast is now redundant

Numpy has become significantly more strict uncovering a few issues

@jenshnielsen jenshnielsen changed the title remove redundant typecast Fix typechecking with newest qcodes and Numpy Feb 10, 2022
@jenshnielsen
Copy link
Collaborator Author

@jpsecher For the QDAC2 this method was typed as returning a sequence but does actually return a numpy array

Unfortunately numpy arrays are currently not considered sequences see numpy/numpy#2776

Do you have a preference if the type signature should change like I have done here or we should convert the output to a list

@jenshnielsen
Copy link
Collaborator Author

@sldesnoo-Delft @eendebakpt

Numpy 1.22 triggers a type checking error in the m4i driver related to from buffer. Since I don't have much context on this driver I have silenced the warning here but perhaps you want to look into a better fix?

@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2022

Codecov Report

Merging #116 (31430f4) into master (b6b5a6c) will increase coverage by 0.03%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage   19.81%   19.85%   +0.03%     
==========================================
  Files         121      121              
  Lines       14703    14709       +6     
==========================================
+ Hits         2914     2920       +6     
  Misses      11789    11789              
Impacted Files Coverage Δ
...trib_drivers/drivers/NationalInstruments/Switch.py 0.00% <0.00%> (ø)
qcodes_contrib_drivers/drivers/Spectrum/M4i.py 34.84% <0.00%> (ø)
qcodes_contrib_drivers/drivers/QDevil/QDAC2.py 96.33% <100.00%> (ø)
...ontrib_drivers/tests/QDevil/test_sim_qdac2_init.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6b5a6c...31430f4. Read the comment docs.

@sldesnoo-Delft
Copy link
Contributor

I will have a look at the M4i driver. This week I already worked on another fix for the driver.

@sldesnoo-Delft
Copy link
Contributor

I just learned that numpy 1.22 requires Python 3.8. However Python 3.8 is not supported by the older Keysight PXI system drivers and firmware. It requires the latest drivers and firmware that have been released in December 2021.
I've not yet had time to test the new firmware and update the drivers.
This might imply a breaking change for people with older licenses that do not allow an update of the firmware.
Dropping support for Python 3.7 would mean that people have to buy new licenses to be able to upgrade to new firmware and drivers.

What's the policy of qcodes regarding the supported Python versions?

@jpsecher
Copy link
Contributor

jpsecher commented Feb 10, 2022

@jenshnielsen thanks, the changes to QDAC2.py look fine.

@jenshnielsen
Copy link
Collaborator Author

@sldesnoo-Delft this does not actually drop 3.7 support just yet but just stops type checking with 3.7. We were going to drop 3.7 exactly because numpy, pandas, xarray ipython and so on has dropped support. Could you copy your comment into microsoft/Qcodes#3889 that may be an important datapoint for delaying the dropping of 3.7 but its nice if we can collect all info there.

@jenshnielsen jenshnielsen merged commit 7fcb326 into QCoDeS:master Feb 10, 2022
@jenshnielsen jenshnielsen deleted the fixtypecheck branch February 10, 2022 10:16
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

5 participants