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 boxcar average option to M4i driver #1509

Merged
merged 19 commits into from
Mar 8, 2019

Conversation

peendebak
Copy link
Contributor

Changes proposed in this pull request:

  • Add option for boxcar averaging
  • Update class docstring
  • Cleanup code by using the __transfer_buffer_numpy method

@WilliamHPNielsen @astafan8

@codecov
Copy link

codecov bot commented Mar 5, 2019

Codecov Report

Merging #1509 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1509   +/-   ##
=======================================
  Coverage   73.81%   73.81%           
=======================================
  Files          92       92           
  Lines       10434    10434           
=======================================
  Hits         7702     7702           
  Misses       2732     2732

@codecov-io
Copy link

codecov-io commented Mar 5, 2019

Codecov Report

Merging #1509 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1509   +/-   ##
======================================
  Coverage    73.8%   73.8%           
======================================
  Files          92      92           
  Lines       10444   10444           
======================================
  Hits         7708    7708           
  Misses       2736    2736

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.

Nice refactoring (apart from the new "features") that reduces code duplication!

qcodes/instrument_drivers/Spectrum/M4i.py Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Outdated Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Outdated Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Outdated Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Outdated Show resolved Hide resolved
qcodes/instrument_drivers/Spectrum/M4i.py Outdated Show resolved Hide resolved
@astafan8 astafan8 added the driver label Mar 7, 2019
@peendebak
Copy link
Contributor Author

@astafan8 Addressed the comments. I am not sure about my solution to fix a mypy error (line 804). Do you have a better alternative?

@astafan8
Copy link
Contributor

astafan8 commented Mar 7, 2019

I'd suggest the following:

sample_ctype: Type
if bytes_per_sample == 2:
    sample_ctype = ct.c_int16
elif bytes_per_sample == 4:
    sample_ctype = ct.c_int32
else:

or a stricter version

sample_ctype: Union[Type[ct.c_int16], Type[ct.c_int32]]

Could you try them and see which one works?

@astafan8 astafan8 merged commit 021eea9 into microsoft:master Mar 8, 2019
giulioungaretti pushed a commit that referenced this pull request Mar 8, 2019
Merge: 1402587 dd5e70a
Author: Mikhail Astafev <astafan8@gmail.com>

    Merge pull request #1509 from VandersypenQutech/feat/m4i_boxcar
@peendebak peendebak deleted the feat/m4i_boxcar branch March 22, 2019 22:30
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

4 participants