this module's name is wrong?

pyvisa / pyvisa-py

A pure python PyVISA backend

https://pyvisa-py.readthedocs.io

MIT License

288 stars 124 forks source link

this module's name is wrong? #253

Closed greyltc closed 4 years ago

greyltc commented 4 years ago

I think https://github.com/pyvisa/pyvisa-py/blob/master/setup.cfg#L2 means this gets installed into my site-packages folder as pyvisa-py and PEP8 tells me that having a hyphen in a module name is a no-no.

The module's folder name in this repo is pyvisa_py, as it should be.

Looks like pyvisa's ResourceManager looks first for a module named pyvisa_py and then falls back to pyvisa-py: https://github.com/pyvisa/pyvisa/blob/master/pyvisa/highlevel.py#L2823-L2828

Should line 2 of setup.cfg be changed to pyvisa_py?

I'm mentioning this because I maintain pyvisa-py's conda-forge feedstock, and this module naming issue has come up there https://github.com/conda-forge/pyvisa-py-feedstock/pull/14

hgrecco commented 4 years ago

This library should not be called by the end user. Therefore, it was named like this on purpose to discourage users to import it.

tonyllevans commented 4 years ago

I'd like to second the name change because this is causing issues interfacing with other packages: https://github.com/pyvisa/pyvisa-py/issues/216

MatthieuDartiailh commented 4 years ago

I changed the name some time ago (for the package in site_packaged not for the project). See PR #238 and related discussion in https://github.com/pyvisa/pyvisa/pull/525 and https://github.com/pyvisa/pyvisa-py/issues/239#issue-641390139. I may have missed something in setup.cfg . Feel free to open a PR.

greyltc commented 4 years ago

@hgrecco

This library should not be called by the end user. Therefore, it was named like this on purpose to discourage users to import it.

Yeah, I get that. Maybe the best way to discourage people from randomly importing it is by adding a line in the docs telling the user this is not something they should be importing? (if there's not one already) EDIT: I put a note into the docs in my PR https://github.com/pyvisa/pyvisa-py/pull/255/commits/7654e506050a3e0a9fc881716fddb57cfac81bcc

I had not noticed #239. This seems like the best reason I've seen for switching the installed module name to a legal one. I'll make a PR and see how it goes.

MatthieuDartiailh commented 4 years ago

The current situation (master) is as follows:

the project is named PyVISA-py (same as in 0.4.1)
the package installed under site-packages is called pyvisa_py, which allows to import it directly even though it is not how it is meant to be used. In 0.4.1 it was pyvisa-py which was indeed causing some issues.
this is in line with what other packages do, see pytest-cov for example.
as a consequence pip will list PyVISA-py as being installed but users can choose consciously to misuse the package by importing pyvisa_py

I personally do not see the need for changing anything at this point on master. #255 will lead to pip listing the project as pyvisa_py which from my point of view is not desirable.

I will close this issue since I consider it fixed on master.