-
Notifications
You must be signed in to change notification settings - Fork 319
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
Dont assume that unit attribute exists #770
Conversation
@jenshnielsen I tested your changes. They do not fix the problem. Looking a bit more at the traceback I think it may have to do with having multiple subplots. I can see that the first plot get's created correctly (out of 2 I am trying to create) but the second one does not. |
Do you get the same traceback? or a different one? Can you inspect how the |
I.e this is how it looks for me for a simple plot of a line graph and a 2d plot in
|
@jenshnielsen Below the traces and the plot.traces. I have to focus on my experiment now but I think this info should be useful. Traceback :
Inspecting traces: |
More readable
|
@AdriaanRol Thanks I will have a look |
Your config object is quite different so the auto scaling will not work as is but I think this should be enough to keep the crash from happening |
@AdriaanRol did you have a chance to test the latest version? |
@jenshnielsen I didn't see you made any further changes. I'll have to check that out. Won't be today though, we are kind of deadline pressed at the moment. On a related note, I think it would be good to add some unittests to the plotting to prevent these kind of issues in the future. I'd say they should only test if the code runs as testing for visual correctness is notoriously hard. If you are interested, I have a travis build script (based on the QCoDeS one) that also includes proper installation of pyqtgraph. |
@AdriaanRol thanks thats fine whenever you have time. Unittesting is great but given that you are using it with a config object that is completely foreign to qcodes I don't see how we could have tested this without better visibility to what you are doing |
@jenshnielsen . I'll try to create a unittest using only the default QCoDeS QtPlot API that reproduces the problem. I think that should provide the visibility that you need and should prevent these kind of problems in the future. |
Sounds good thanks |
@jenshnielsen I've made a new PR containing a self contained test using only the default QCoDeS API that also contains all the work from your branch. Looking forward to your input. |
Super-seeded by #805 |
Fixes #769 .