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.

Sign up for GitHub

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

Jump to bottom

Update agilent drivers to conform to docs standard #4371

Merged
merged 11 commits into from Aug 1, 2022
Merged

Update agilent drivers to conform to docs standard #4371

merged 11 commits into from Aug 1, 2022

Conversation

jenshnielsen
Copy link
Collaborator

Updates to implement #4368 for Agilent. Closes #4365

@mgunyho Could you test the new driver. I have updated the name of the variable and gotten rid of the not needed deg2rad converters and replaced them with numpy

I also suggest that we get rid of the autogenerated docs for the instruments and replace them with something like this.
I would like to add autodocsumm for these modules but am facing the same issue as in #4273

@codecov
Copy link

codecov bot commented Jul 7, 2022 •
edited

Codecov Report

Merging #4371 (ba08fa6) into master (34e667c) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head ba08fa6 differs from pull request most recent head afe0f4a. Consider uploading reports for the commit afe0f4a to get more accurate results

@@            Coverage Diff             @@
##           master    #4371      +/-   ##
==========================================
- Coverage   68.38%   68.36%   -0.02%     
==========================================
  Files         283      276       -7     
  Lines       31043    31024      -19     
==========================================
- Hits        21229    21210      -19     
  Misses       9814     9814

@jenshnielsen jenshnielsen mentioned this pull request Jul 16, 2022
Closed
This was referenced Jul 25, 2022
Closed
Merged
@jenshnielsen jenshnielsen force-pushed the driver/agilent_cleanup branch 2 times, most recently from c61340e to dea02a6 Compare July 25, 2022 17:16
@jenshnielsen
Copy link
Collaborator Author

Will postpone merging this until we verify this fixes #4365 and make a decision on #4273 since the docs are hard to navigate without autosummary (due to all the new classes etc)

@jenshnielsen jenshnielsen mentioned this pull request Jul 29, 2022
Merged
@jenshnielsen jenshnielsen added the docs Related to docs improvements label Aug 1, 2022
@mgunyho
Copy link
Contributor

mgunyho commented Aug 1, 2022 •
edited

I tested with an E8257D unit that the new driver works, I tried snapshot(update=True) and setting output_enabled, power, frequency and phase, I checked on the device that they were set. So this fixes #4365

@jenshnielsen
Copy link
Collaborator Author

@mgunyho Thanks

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 1, 2022

Build succeeded:

@bors bors bot merged commit 308f556 into microsoft:master Aug 1, 2022
@jenshnielsen jenshnielsen mentioned this pull request Aug 2, 2022
Closed
@jenshnielsen jenshnielsen added this to the Public api rewrite milestone Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astafan8 astafan8 astafan8 approved these changes

Assignees
No one assigned
Labels
docs Related to docs improvements
Projects
None yet
Milestone
Public api rewrite
Development

Successfully merging this pull request may close these issues.

Typo in the name of the Agilent E8257D driver
3 participants
@jenshnielsen @mgunyho @astafan8