Updates and improvements for Python 3 branch

nanophotonics / nplab

Core functions and instrument scripts for the Nanophotonics lab experimental scripts

GNU General Public License v3.0

38 stars 15 forks source link

Updates and improvements for Python 3 branch #85

Closed rwb27 closed 4 years ago

rwb27 commented 4 years ago

Nice job on making it work in Python 3 :) I've added a few fixes that are useful in my lab at Bath:

Updated instrument.camera.opencv to use the latest version of opencv-python (the cv2.cv submodule has been removed)
Updated the instrument.spectrometer.seabreeze wrapper to use the latest version of seabreeze (they renamed a bunch of stuff, which is a bit annoying - functionality seems the same, though).
Added BPC203 benchtop Piezo controller to the instrument.stage.thorlabs-kinesis module
Added a helpful error message to the Kinesis wrapper if it doesn't import correctly.

NB the changes to seabreeze will break systems with the old SeaBreeze and require an update before it works properly again. It would be possible to work around this with some clumsy exception handling if necessary.

YagoDel commented 4 years ago

Thanks! Perhaps @mjh250 would like to have a test before we merge it, but it looks good to me

rwb27 commented 4 years ago

cool :)

By the way, are there issue threads or something documenting the Python 3 progress? The data browser didn't seem to be working, but I'm not sure if that's just me or a Python 3 specific thing. I might be able to steal some time to work on updating the datafile handling stuff if needed, as I'm intending to use that in my group.

eoinell commented 4 years ago

Would it be possible to split this merge into opencv and 'other' updates?

We're currently testing the python3 branch on setups and are keen to incorporate the newest opencv, however the majority of users who are still on 2.7 for the moment need the old seabreeze drivers.

Or could the seabreeze updates be incorporated easily into master?

YagoDel commented 4 years ago

@rwb27 About documenting Python 3 progress, we're currently having a discussion on Slack about it. Ideally we'll move the conversation to GitHub issues soon, but we need a bit of coordination first

@eoinell In terms of ease to incorporate seabreeze updates into master, I had a look through the usage of seabreeze in master, and everything goes through the seabreeze.py file. So everything can be directly incorporated, assuming the changes in this merge #85 cover all the updates to the DLL. For the majority of users, they will need to update their drivers, but they can still choose to use Python 2.7

YagoDel commented 4 years ago

@rwb27 we've started using GitHub issues. And the DataBrowser should be working (at least the test runs on it)

YagoDel commented 4 years ago

@eoinell About the seabreeze working for both new and old drivers. If I'm not wrong, the difference between the current master and this pull request is the DLL version (1 or 2, respectively, see https://oceanoptics.com/api/seabreeze/). If that's the case, you could add a switch that checks the API of the DLL (maybe using the getAPIVersionString function of the DLL, https://oceanoptics.com/api/seabreeze/class_sea_breeze_wrapper.html). And then use that switch for the three lines of code that are different between the two APIs. I could write it, but I don't have SeaBreeze spectrometers at my lab to test it

eoinell commented 4 years ago

I'm afraid that's a little beyond me. I can test it, although it's probably best if we just update all the pc's OO drivers.

For now, I'm happy to go ahead with this merge and try the new version of openCV and see if that fixes https://github.com/nanophotonics/nplab/issues/93

rwb27 commented 4 years ago

I'll take another look at this as soon as I get time, I think I updated everything in the seabreeze wrapper, but it wouldn't surprise me if I missed something that I wasn't using. As the calls are pretty similar, it oughtn't be a huge job to support both, though I don't have DLL v1 here to test.