search

pyvisa / pyvisa-py

A pure python PyVISA backend
https://pyvisa-py.readthedocs.io
MIT License
282 stars 120 forks source link

Architecture extension issue #239

Open MatthieuDartiailh opened 4 years ago

MatthieuDartiailh commented 4 years ago

Trying to add mypy validation to pyvisa-py following https://github.com/pyvisa/pyvisa/pull/511, I hit a wall. Namely mypy cannot run on a package that has an "invalid" name (and it seems that flake8 is similarly unhappy). Furthermore some people complained about the poor access to the internal of pyvisa-py. I know there are workarounds but still.

This is honestly very annoying going forward. I can see multiple paths forward and I would like to get feedback from the as many people as possible (@hgrecco, @greyltc, @twam, @KOLANICH). First I do not propose to change the name of the project, so no disruption to PyPI packages. Changes would only affect the internal structure and possibly PyVISA.

I personally find the second option more attractive. Since I am not aware of any other extensions than the ones we host (I do not consider pyvisa-sim maintained at this point), we can handle all the disruption internally and just say to people to properly update both packages.

WDYT ?

KOLANICH commented 4 years ago

IMHO - backends should be tied not by name, but via setuptools entry_points

create a backends namespace package in pyvisa

is a terrible solution which is badly broken. I have used it in several of my package (only to just clean up the mess, I mean not to have several dirs in site_packages, but it turned out it is better to have terrible mess like that than being unable to use pip install -e .).

So, I propose renaming + backends discovery using entry points (I also have found a mechanism to store metadata in them (I have unsuccesfully tried to standardize it (so it'd be possible to standardize some subset of metadata, i.e. I use metadata to mark issues in the backends (i.e. some backends may be licensed under GPL (I personally don't license my works under GPL unless I am forced by an author of some dependency it is infeasible to replace), so it is legal to load them only in apps and libs under GPL, so I if I am not allowed use the entry point (when I load an entry point its module is imported) to discover the fact I am not allowed to use it, I workaround it by storing metadata, to read metadata (stored as a part of its name, it's a hack) I don't have to load the entry point) and their priority, and for setuptools I have also proposed to use the mechanism to allow setuptools plugins to request some args to be passed), but setuptools maintainers have opinion that it shouldn't be standardized and every lib should use the way to store metadata convenient to it), basically you add a delimiter to entry point name (I use @) and after it put JSON).

MatthieuDartiailh commented 4 years ago

Thanks @KOLANICH I did not realize that PEP420 support in editable install was not really there. I should I have guessed though. So no matter, the only solution left is to change the package name in site-packages. I agree that name based discovery is not the nicest.

I have used setuptools entry points successfully in the past (just require some care when dealing with conda packages) and would be open to that option. Any other opinion ?

MatthieuDartiailh commented 4 years ago

Also @KOLANICH would you have time to propose a patch using setuptools entry points ?

KOLANICH commented 4 years ago

Probably no, but you can look at my kaitaiStructCompile lib that uses the hack with the name of entry points. Also File2Package. Feel free to borrow code from them.

greyltc commented 4 years ago

Hopefully #255 does exactly your first suggestion @MatthieuDartiailh:

  • first simply rename pyvisa-py in site-packages to pyvisa_py: This is is minimally disruptive.
MatthieuDartiailh commented 4 years ago

255 is not necessary, my first suggestion has already been implemented as part of #238. See https://github.com/pyvisa/pyvisa-py/issues/253#issuecomment-687666704 for a more detailed explanation. The only remaining point but that I see as low priority is the use of setuptools extension points a s a mean to detect backends.