LabPy / lantz_drivers

BSD 3-Clause "New" or "Revised" License
0 stars 4 forks source link

Package organisation #1

Open MatthieuDartiailh opened 9 years ago

MatthieuDartiailh commented 9 years ago
hgrecco commented 9 years ago

I think by vendor is the organization that we all have reached in our packages. One package/folder per vendor is the right way to go.

They only way to address the third point is to have summary file with the most important information. This could be easily done automatically with script that the developer should run (or linked to a commit hook)

MatthieuDartiailh commented 9 years ago

Ok so we agree about teh general organisation. I guess automating the third point is the easiest way to go, we will to thing about it. So only remains the case of a single incrstrument existing under two vendor names (I have microwave sources which are agilent or keysight according to when they were bought). We could implement the driver in one vendor and create a dummy subclass in the other folder that is perhaps the easiest solution.

alexforencich commented 9 years ago

Storing by vendor sounds like the consensus. Seems like some form of automation would be a good idea.

As for multiple vendor names, that is a bit more problematic. One method would be to just list everything under the current vendor name and hope it doesn't change again (sigh...HP->Agilent->Keyshite->???). Another method might be to list items under the original brand only (originally HP -> list under HP, even if there are Agilent branded units as well). Or we could do that and then create an alias of some sort under the current brand as well.

hgrecco commented 9 years ago

I prefer to have it named as it is seen in the device. The rationale is that a person that is standing in front of an instrument can directly find the right driver without knowing the history of the company.

If the same device is then sold with another brand/model then we just create an new python file importing the original class with the new name (in a new packages).

alexforencich commented 9 years ago

What about file naming? For common base classes that are used by multiple drivers, I suggest storing these in a folder called 'common'. I think the driver dev branch has 'standards' right now, but that to me indicates something along the lines of calibration standards or something along those lines and not driver components. Thoughts?

For instrument models, it is common to start with a number (e.g. 8340A), which is a no-no for a Python module name. So what is the most reasonable solution for that? I think duplicating the manufacturer name makes sense (e.g. lantz_drivers.agilent.agilent_8340A), but adding some other standard prefix may also make sense (e.g. lantz_drivers.agilent.model_8340A). Whatever we do should be a standard though, so any prefix applied would also be attached to things that don't start with a number - so you would get things like lantz_drivers.agilent.agilent_DSA91304A or lantz_drivers.agilent.model_DSA91304A. What about capitalization and delimiters? All lowercase with an underscore (agilent_dsa91304a), lowercase with uppercase model number with or without underscore (agilent_DSA91304A or agilentDSA91304A) or something else?

Also, is there a clean solution to move the actual driver object up one level in the hierarchy? As in, it would be nice to be able to import lantz_drivers.agilent.agilent_8340A and use that directly instead of having to import lantz_drivers.agilent.agilent_8340A.agilent_8340A or lantz_drivers.agilent.agilent_8340A.driver or similar. In python-ivi, this was done by importing all of the classes in the parent init.py, but this is rather tedious.

hgrecco commented 9 years ago

The convention that we had until previous version of Lantz is the following:

  1. One folder for each company
  2. The folder is a (sub)package, contains and init file which imports and exposes ALL the driver classes in the package. (This can be done manually or automated)
  3. The classes are named according to the module using Python PEP8. When the model starts with a number, we prefix with the first letter of the company. I am fine to choose another (short) prefix.
  4. The number of modules in the package is really dependent on what makes sense for the company.
alexforencich commented 9 years ago

If we can come up with a simple way to automate the import process, that would be great. However, it seems to me that the current setup is to only import things explicitly and not to import whole trees. I'm not sure how this would map on to importing all driver modules in init.py files.

I think adding prefixes in a non-consistent manner is not such a good idea. It would not be difficult to imagine a company releasing some products with numeric models, and some that happen to be prefixed with a single letter that happens to be the same as the first letter in the company name. Keysight has quite a few products with part numbers like this - Exxxx, Bxxxx, Nxxxx, Uxxxx, etc. There is no Kxxxx right now, but if they do release something like that in the future it could conflict with something that has a pure numeric model number.

MatthieuDartiailh commented 9 years ago

I agree with @alexforencich about the prefix issue we should be consistent.

I am not a big fan of importing whole subtrees as I fear that the metaclasses magic will slow down the import. We could perhaps simply have factories in init.py with nested imports.

alexforencich commented 9 years ago

Well, if you know how to write said factories, then that sounds like it could be the most reasonable option.

Let's figure out the prefixes so I can start writing some drivers. We're throwing a couple of undergrads at this over here at UCSD, so it would be nice if we can get these last few blocking kinks worked out soon.

hgrecco commented 9 years ago

I think that we are over-engineering. Most instrumentation applications last long enough so that the import time is negligible. And we can always optimize that later. I think the important thing is to provide a convenient way, which in my opinion is:

from lantz_drivers.company import driverclass

and not:

from lantz_drivers.company.module import driverclass

This requires just importing the driver classes into the company __init__.py, which is something that you write just once and if that is very tedious you can just write a script to do it for you.

Regarding the prefixes. I am fine to use Model as in Model8340A.

MatthieuDartiailh commented 9 years ago

My point with factory would be to have :

def GS200(*args, **kwargs):
    from . gs200 import GS200
    return GS200 (*args, **kwags)

Which we could also automatize.

But I am not sure it is true good idea.

MatthieuDartiailh commented 9 years ago

I am fine with model as a prefix.

hgrecco commented 9 years ago

My suggestion is that we can always do it later. Let's first do it in the most straightforward manner, we can always drop your solution later (which is fine for me) without any backwards compatibility problem.

Having said this, I think a better idea for the future can be something like the following in lantz_drivers/__init__.py

def get_driver_class(manufacturer, model):
     # here we look into a YAML or json  file with a tree structure, for example:
     #    agilent:
     #       8340A: lantz_drivers.agilent.somemodule.Model8340A
     #
     # This file is kept in sync with a commit hook, but you can also build upon installation

     return driver_class # which is imported according to the YAML/JSON file

I played in the past with this option and was really nice as you could easily have a single driver (without any alias classes) for whole bunch of devices that are basically the same. The import performance is really good because it only reads lantz_drivers/__init__.py and the YAML/JSON file. The drivers are really imported on demand. In any case, I think this can be done latter.

alexforencich commented 9 years ago

I think Model is OK when an instrument starts with a number, but it seems awkward to me when it doesn't. For example, Model8340A and ModelMSO7104A vs. Agilent8340A and AgilentMSO7104A.

alexforencich commented 9 years ago

So, if we use that get_driver_class method, can we no longer use import?

hgrecco commented 9 years ago

Why not? You can always use import.

alexforencich commented 9 years ago

So how would you use that get_driver_class method, then? import lantz_drivers.get_driver_class('Agilent', '8340A')? Or am I misunderstanding how that would work?

MatthieuDartiailh commented 9 years ago

I really like the get_driver_class solution, as it can also allow to easily list all availables drivers. Model does look awkward to add to every name, repeating the manufacturer might be better.

hgrecco commented 9 years ago

If you want the usual stuff, you do:

from lantz_drivers.company import Model38

But If you consider that the import takes too long for your script (which as I said before, is rare), you do:

from lantz_drivers import get_driver_class
C = get_driver_class('company', 'Model38')
hgrecco commented 9 years ago

I really hate doing

from lant_drivers.company import Company292939

but if that is the majority decision then I am fine with it.

MatthieuDartiailh commented 9 years ago

This won't work wih an init.py full of imports. To really gain tme you need an empty init.py.

hgrecco commented 9 years ago

@MatthieuDartiailh What do you mean? lantz_drivers/__init__.py does not need any extra import

MatthieuDartiailh commented 9 years ago

No lantz_drivers.company needs to be empty otherwise all imports are executed when importing lantz_drivers.company.model.

hgrecco commented 9 years ago

You can import the module without the __init__.py as long as you do not share anything.

hgrecco commented 9 years ago

But again the nice thing, in my opinion, is the aliasing and make it programatic. It is really useful for more high level appplications and when instantiating devices according to a configuration file

alexforencich commented 9 years ago

Well, autocomplete in ipython is also nice. It's certainly something to consider, though. I can definitely see how it could be very useful.

MatthieuDartiailh commented 8 years ago

@alexforencich you were not happy with using standards for storing the "standard" base classes. You would prefer specifications (staying close to the IVI terminology). If we want to keep direct import possible I would prefer to have an drivers.py file per vendor in which all drivers are imported but keep the init.py file empty as otherwise importing any driver would lead to building all the classes (as the init.py found in keysight is executed when importing lantz.drivers.keysight.model_E8685A). I will try to implement the get_driver_class function based on .yaml files, as I like aliasing the capabilities it opens and the possibility to use only the bare model name without any restrictions.

alexforencich commented 8 years ago

Another thing that could possibly be included in .yaml files could be identifying metadata, like USB vendor and product IDs, so that it would be possible to automatically detect a device and load the correct driver. I'm not sure if this would be possible for all devices; it may not be feasible for devices that do not support the idn command.

And how about just making a folder 'base' for all of the base classes? Short, simple, to the point. Then you can have lantz.driver.base.dc_source.

MatthieuDartiailh commented 8 years ago

I am fine with base too. As long as we do not conflict with a vendor name we are fine. Things like usb vendor and product id can be specified in the PROTOCOLS class attributes. I should expand the docs on this. Your idea is interesting but will be indeed limited to instruments supporying *IDN.

alexforencich commented 8 years ago

Well, the idea I had about putting them in the yaml files is that then you don't need to load the whole driver to get the value. There is probably something to be said for generating the yaml files from the python files in some way, though.

alexforencich commented 8 years ago

Also, what about some 'standard' features that should be supported by all instruments? Things such as manufacturer, model, serial, firmware version, reset, error query, self test, etc. IVI has a whole pile of these, but I think most of them aren't really all that necessary.

MatthieuDartiailh commented 8 years ago

I do plan to have a look at the base IVI stack but I prefer to be cautious with what we impose on instruments (as some stupid one may break). But I will definitively add some base classes to take care of that. In dc_sources I started an scpi packages for that kind of things, I will try to expand it.

MatthieuDartiailh commented 8 years ago

Looking at the Inherent capabilities described in IVI:

I can create abstract classes in base and some concrete in scpi (should scpi be a root level package or live in base ?). For identity, I can perhaps take advantage of stringparser to let the user simply provide the template for the IDN answer as a class attribute.

alexforencich commented 8 years ago

That's basically the conclusion that I came to - some basic metadata - manufacturer, model, serial, firmware, possibly driver version information - and some standard actions - reset, self_test, error_query. I'm not sure if there is anything else there that makes sense.

As for SCPI, this is not a base class as it implements commands, so it should not be in the base folder. However, it could make sense to add corresponding base classes for the implemented features/actions. A top-level SCPI folder would probably make the most sense. I'm not sure about the ieee488 stuff, though, since it's common to multiple instruments but it isn't exactly SCPI. Maybe we can have a top-level folder 'common' for things like that. Or we could just throw it in the SCPI folder.

MatthieuDartiailh commented 8 years ago

We could also have a common package and inside a ieee488 for * commands and a subpackage for scpi.

alexforencich commented 8 years ago

That works too. Now, the next question is how best to organize the features. All top-level, or would it make sense to organize a few things into subsystems? As in, would it make sense to implement inst.model, inst.error_query, inst.reset, etc. or inst.utility.model/inst.meta.model, inst.utility.error_query, inst.utility.reset, etc.?

MatthieuDartiailh commented 8 years ago

I think that reset, self_test, and query_error (better than error_query as this will be an action) should be at the root level, there are not that many and I don't think that list will grow. For the identity I think it would make sense to have a subsystem to avoid polluting too much the root, and I think calling it identity would be fine actually.

MatthieuDartiailh commented 8 years ago

For the identity subsystem, I think it is fine to have more field than the instruments can potentially provide informations for (and we could simply leave them blank (returning '')), what bother me is that once again some instruments does not have reset, self_test, or error_reading capabilities (I do have such a case each command return an execution result but there is nothing such as error reading). I am in favor of keeping reset, self_test, and error_query in common but not in base. Actually I think that we won't be able to do everything with base classes but we will also need some guidelines (saying that name should be used) otherwise the number of base classes will blow up (for examples in DC sources I am currently in the nightmare of determining what should be base as basically no instrument I found (even a keysight E3631A) follow IVI recommendations).

alexforencich commented 8 years ago

Yeah, that's one of the issues with base classes - not everything matches up exactly, so either you have to have a very large number of very small base classes so you get the required granularity, or you have to provide some way to not implement particular portions of a base class (no feature/action at all), or leave unsupported features unimplemented (feature returns none/action does nothing or raises exception, etc). None of which are ideal.