Reorganize code in servers repo

[maffoo]

The servers repo is a mishmash of servers, scripts and library code, without a very coherent organization. This makes it hard to know how to get it on your python path, and also makes it hard to know inside code itself how to import and use other code.

From the discussion of #108:

ghzdac is a package containing library code for the deconvolution; this code should be importable from elsewhere, so it should have an init.py and it should be on the python path.

ghzdac_cal contains runnable scripts for doing calibration; these scripts should not be in a python package, given python's separation of scripts from library code, and so this should not have an init.py file. Or at the very least, the scripts should be refactored to put all the logic in a python package, and just have runnable stubs in a separate scripts directory (which is not a python package) that just imports and runs the appropriate library code. The stubs could also do some path manipulation to make sure that the corresponding package directories from the same git checkout are on the python path; this is a concession we've discussed for people who are on windows and want scripts to be double-clickable in a gui environment on a shared machine that may have different people's personal checkouts and so the system python path may point at the wrong location.

Finally, GHzDACs contains server code as well as other miscellanea (looks like there are some scripts in there, etc.) Again, anything that should be top-level runnable should be refactored into library code and a script stub in a non-package directory. I think the server itself should also receive the same refactoring treatment, since it needs to be top-level runnable by the node.

So, roughly speaking, I propose the following structure for the servers repo:

ghzdac/
    __init__.py
    adc.py
    calibrate.py
    ...

scripts/
    GHz_DAC_bringup.py
    ...

servers/
    ghz_fpga_server.py
    ...

Note that we would remove the top-level init.py file in the servers repo; this whole thing is not a python package. Also note that the scripts and servers folders do not have init.py files; these are also not python packages. The servers checkout would no longer have to be put inside an extra directory when checked out and the checkout folder itself could be added to the pythonpath (at least on non-shared machines or in virtualenvs) and this would make the code in the ghzdac package available. Also note that the ghzdac package as I've sketched it contains both the calibration and server code mashed together; we'd probably want to structure that differently, but I think having a clean separation between scripts and library code (+ tools like pycharm) makes that easier.

[ejeffrey]

Is it valuable to be able to import the actual server code? We could put 99% of the server code in the package directory, and then just make stubs with the node info block and calls to util.runServer(). I don't know if this would actually get us anything useful.

Evan

On Mon, Apr 20, 2015 at 10:15 AM, Matthew Neeley notifications@github.com wrote:

The servers repo is a mishmash of servers, scripts and library code, without a very coherent organization. This makes it hard to know how to get it on your python path, and also makes it hard to know inside code itself how to import and use other code.

From the discussion of #108 https://github.com/martinisgroup/servers/issues/108:

ghzdac is a package containing library code for the deconvolution; this code should be importable from elsewhere, so it should have an init.py and it should be on the python path.

ghzdac_cal contains runnable scripts for doing calibration; these scripts should not be in a python package, given python's separation of scripts from library code, and so this should not have an init.py file. Or at the very least, the scripts should be refactored to put all the logic in a python package, and just have runnable stubs in a separate scripts directory (which is not a python package) that just imports and runs the appropriate library code. The stubs could also do some path manipulation to make sure that the corresponding package directories from the same git checkout are on the python path; this is a concession we've discussed for people who are on windows and want scripts to be double-clickable in a gui environment on a shared machine that may have different people's personal checkouts and so the system python path may point at the wrong location.

Finally, GHzDACs contains server code as well as other miscellanea (looks like there are some scripts in there, etc.) Again, anything that should be top-level runnable should be refactored into library code and a script stub in a non-package directory. I think the server itself should also receive the same refactoring treatment, since it needs to be top-level runnable by the node.

So, roughly speaking, I propose the following structure for the servers repo:

ghzdac/ init.py adc.py calibrate.py ...

scripts/ GHz_DAC_bringup.py ...

servers/ ghz_fpga_server.py ... Note that we would remove the top-level init.py file in the servers repo; this whole thing is not a python package. Also note that the scripts and servers folders do not have init.py files; these are also not python packages. The servers checkout would no longer have to be put inside an extra directory when checked out and the checkout folder itself could be added to the pythonpath (at least on non-shared machines or in virtualenvs) and this would make the code in the ghzdac package available. Also note that the ghzdac package as I've sketched it contains both the calibration and server code mashed together; we'd probably want to structure that differently, but I think having a clean separation between scripts and library code (+ tools like pycharm) makes that easier.

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/109.

[DanielSank]

I import stuff from the GHzDACs server package all over the place (even in pyle GASP!). It's really really useful to have a one stop shop for DACifying data, getting packets for SRAM/register writes, and all kinds of debugging for the FPGAs.

[ejeffrey]

Isn't that stuff already in a library? If not, it can/should be moved there. I am talking about moving the actual LabradServer object into a library.

On Mon, Apr 20, 2015 at 10:37 AM, Daniel Sank notifications@github.com wrote:

I import stuff from the GHzDACs server package all over the place. It's really really useful to have a one stop shop for DACifying data, getting packers for SRAM/register writes, and all kinds of debugging for the FPGAs.

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/109#issuecomment-94518551 .

[DanielSank]

Oh, yeah, misunderstood you.

[maffoo]

I think it depends on the server. For the data vault, for example, we could probably refactor things between the single- and multi-headed versions so that they even share the same server class, and just wire it up to different backends. We haven't quite gone that far even in the current refactor that's underway, so there's still a certain amount of duplication between the two versions. But for the most part, I think it would be fine for the actual server class to live in the 'stub' file in the servers directory. We'd basically just move most of the server scripts there and reconfigure the nodes to look in the right place.

One thing to note is that most of the server scripts create a top-level server object in the module, e.g.:

__server__ = MyServerClass()

if __name__ == '__main__':
    import labrad.util
    labrad.util.runServer(__server__)

This was a remnant from the early days when the node tried to use the twisted plugin system to locate servers, and that plugin system requires that plugins be created as module-level objects so that it can inspect all imported modules to find plugin instances. We should get rid of this and just instantiate and run the servers directly:

if __name__ == '__main__':
    import labrad.util
    labrad.util.runServer(MyServerClass())

But I agree that for the most part the server class just be defined right there in the same file.

[maffoo]

Note that this could also provide a solution to #84. Installing the repo would mean cloning it, adding it to python path, and configuring the node to scan the servers directory.

[DanielSank]

Note that this could also provide a solution to #84. Installing the repo would mean cloning it, adding it to python path, and configuring the node to scan the servers directory.

+1

...by which I actually mean "+ a bazillion"

[DanielSank]

I've never understood the preference for organizing by file "type". For example, folks seem to like

scripts/
  peel_banana.py
  turn_on_blender.py
servers/
  banana_peeler_server.py
  blender_server.py

where I would do (roughly)

readme.md  # Tells you what to put in each directory.
peeler/
  server.py
  peel.py  # This is a script
blender/
  server.py
  blend.py  # this is a script

Before I propose how I think we ought to organize the servers repo, can someone comment on the difference between these two? My thinking is that the second way is a lot more like an object oriented language where the directories are classes and the files are methods, which makes sense.

[maffoo]

Python distinguishes between importable code and scripts in a few ways, the most important of which is that you can't do relative imports if you invoke a python file as a script, whereas they work fine in modules that you import from other places. If we ban all relative imports, then in principle we could lump them together and mix scripts in with library code. But I think the separation is still useful, because you can set things up roughly as follows:

lib/
  pkg_foo/
    foo.py
    foo_helper.py
  pkg_bar/
    bar.py
scripts/ # or maybe bin/
  do_something_with_foo.py
  do_something_with_bar.py

Now you would add lib to your PYTHONPATH and could add scripts to your PATH. Also note that the stuff in scripts should be very simple, basically just an import and a main method:

#!/usr/bin/python
from pkg_foo import foo

if __name__ == '__main__':
    foo.do_something()

[DanielSank]

Ah, ok, as long as all of the code for things like the DAC bringup is actually in lib/, and the thing in scripts/ is just a main method which invokes a function in the lib code, I'm down with this. In this world the scripts directory contains nothing more than conveniently executable wrappers around some of the conents of the lib directory, as you say.