OxfordInstruments Proteox driver #301
Conversation
astafan8
changed the title
There was a problem hiding this comment.
@abioxinst Thanks for the contribution and sorry for the slow reply. Overall, we are very happy to see this driver added here. I think we would like to improve the infrastructure a bit so that users can consume the code directly without modifying the src and having to clone the repo directly. I have tried to outline some of the changes below but I am happy to work more closely with you to see how this can be done
| @@ -0,0 +1,3 @@ | |||
| [submodule "src/qcodes_contrib_drivers/drivers/OxfordInstruments/decsvisa"] | |||
| path = src/qcodes_contrib_drivers/drivers/OxfordInstruments/decsvisa | |||
| url = git@github.com:decs-visa/decsvisa.git | |||
There was a problem hiding this comment.
Would it be possible to release decsvisa as a python package? We can probably help with advice on how to setup the infrastructure to make this possible.
Until then I think we should change the directory to _decsvisa to make it clear that this is not public api
There was a problem hiding this comment.
We would like to keep decsvisa as a submodule for the meantime, as the idea is users are able to add additional commands in the command_dictionary. We have renamed this directory to _decsvisa to make it clear that this is not public API as you suggested.
There was a problem hiding this comment.
I would prefer not to have images in src folder. Are these needed? It seems like the same images are in the example folder?
There was a problem hiding this comment.
The images in the src folder have been removed. All images are now in the example folder.
| "import qcodes as qc\n", | ||
| "from qcodes.logger.logger import start_all_logging\n", | ||
| "\n", | ||
| "sys.path.append(\"../../src/qcodes_contrib_drivers/drivers/OxfordInstruments/\")\n", |
There was a problem hiding this comment.
qcodes_contrib_drivers is a package so rather than adding it to path like this one should replace the imports with
from qcodes_contrib_drivers.drivers.OxfordInstruments.Proteox import oiDECS
There was a problem hiding this comment.
The file has been updated to replace paths with imports for qcodes_contrib_driver.
| from src.decs_visa_tools.decs_visa_settings import PORT | ||
| from src.decs_visa_tools.decs_visa_settings import HOST | ||
| from src.decs_visa_tools.decs_visa_settings import SHUTDOWN | ||
| from src.decs_visa_tools.decs_visa_settings import WRITE_DELIM |
There was a problem hiding this comment.
Rather than modifying the path again this should be imported as a module. As mentioned elsewhere preferably decs_visa_tools should be a package but until then the imports should be
from qcodes_contrib_drivers.drivers.OxfordInstruments.Proteox.decsvisa.src.decs_visa.src.decs_visa_tools.decs_visa_settings import PORT
There was a problem hiding this comment.
The file has been updated to replace paths with imports for qcodes_contrib_driver.
| ````python | ||
| if running_on.startswith("Windows"): | ||
| print(f"Running on {running_on} - start subprocess without PIPEd output") | ||
| subprocess.Popen(["python", decs_visa_path]) |
There was a problem hiding this comment.
Rather than having to start the server manually like this one could add an entry point script that can be run directly. See for example this server for the Dynacool PPMS https://github.com/microsoft/Qcodes/blob/main/pyproject.toml#L124
There was a problem hiding this comment.
decsvisa can be used independently from QCoDeS, which requires the user to modify some settings in decsvisa.
We are happy to look into how we might be able to automatically start this server for QCoDeS users, but still keep it flexible for other users.
For the meantime, starting the server with the subprocess package seems to work well for accommodating both use cases.
driverdirectory, we have changed the directory name fromOxfordtoOxfordInstruments.OxfordInstruments, you will now find our QCoDeS driver,Proteox.py, and aREADME.mdfile to explain how to use it.Proteox.py, our decs-visa application is required. Therefore, this has been added as a submodule within theOxfordInstrumentsdirectory.docs/examplesdirectory, you will find an example Jupyter Notebook, titledOxfordInstruments_Proteox.ipynb.