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

SignalHound_USB_SA124B print statement. #415

Closed
giulioungaretti opened this issue Dec 12, 2016 · 2 comments · Fixed by #1154
Closed

SignalHound_USB_SA124B print statement. #415

giulioungaretti opened this issue Dec 12, 2016 · 2 comments · Fixed by #1154
Labels
Milestone

Comments

@giulioungaretti
Copy link
Contributor

@elrama- @AdriaanRol

The print statement in this driver are pretty interesting!
Is there any reason for https://github.com/QCoDeS/Qcodes/blob/master/qcodes/instrument_drivers/signal_hound/USB_SA124B.py#L271 ?

I am thinking of replacing the three print statement with a single log.error with a multi line string!

@giulioungaretti giulioungaretti added this to the Logging milestone Dec 12, 2016
@AdriaanRol
Copy link
Contributor

@giulioungaretti I agree on using logging for these things. My preference would be logging.warning but my guess is you want to build in some logging module for QCoDeS?

In any case @elrama- is on holidays now so I don't expect him to react, but I do remember that he spent some time on these error messages as it took some time to figure out how the driver worked.

Good to see driver improvements happening 👍

@giulioungaretti
Copy link
Contributor Author

@AdriaanRol https://github.com/QCoDeS/Qcodes/pull/416/files#diff-227f5982ce997835113c6c7997170267R271 is my proposal :D
If you raise after, it should be an logging.error rather than a warning but that it's more of a decision the driver artist has to make.

Yes you guessed right :D !

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 a pull request may close this issue.

2 participants