roboticslab-uc3m / yarp-devices

A place for YARP devices
https://robots.uc3m.es/yarp-devices/
9 stars 7 forks source link

Python packaging issues #201

Closed David-Estevez closed 1 year ago

David-Estevez commented 5 years ago

I'm trying to recycle the code for the grabbercontrols2gui in other program in a clean and non-hacky way. The best way to do so would be to install the Python code in this repo and the import it in my code. But it seems that the current name of the package (yarp-devices) is not a valid Python module identifier. My proposal is to change it to something like rlyarpdevices or similar to avoid the use of the forbidden '-' character.

Related to this, although it might not be an issue, is the fact that I'm not able to install the python gui script with Python 3.4 or higher, as it is explicitly forbidden in the CMakeFile. Does anyone remember the reason of this constraint?

jgvictores commented 5 years ago

I'm trying to recycle the code for the grabbercontrols2gui in other program in a clean and non-hacky way. The best way to do so would be to install the Python code in this repo and the import it in my code. But it seems that the current name of the package (yarp-devices) is not a valid Python module identifier. My proposal is to change it to something like rlyarpdevices or similar to avoid the use of the forbidden '-' character.

Could you please go for roboticslab_yarp_devices?

  1. It's clear and explicit (I've found myself explaining what rl means)
  2. In consumer code, we can always import roboticslab_yarp_devices as rlyd or import roboticslab_yarp_devices as rlyarpdevices
  3. Similar to https://github.com/asrob-uc3m/yarp-devices/blob/e4e26ab4f7c739060688342b2f6ce513e22c3a14/bindings/asrob_yarp_devices.i#L15

Related to this, although it might not be an issue, is the fact that I'm not able to install the python gui script with Python 3.4 or higher, as it is explicitly forbidden in the CMakeFile. Does anyone remember the reason of this constraint?

Sorry, I do not know the reason.

PeterBowman commented 5 years ago

Related to this, although it might not be an issue, is the fact that I'm not able to install the python gui script with Python 3.4 or higher, as it is explicitly forbidden in the CMakeFile. Does anyone remember the reason of this constraint?

See https://github.com/roboticslab-uc3m/yarp-devices/issues/174. We should now be safe to remove that Python version constraint from the CMake code thanks to https://github.com/roboticslab-uc3m/yarp-devices/pull/195.

David-Estevez commented 5 years ago

Could you please go for roboticslab_yarp_devices?

  • It's clear and explicit (I've found myself explaining what rl means)
  • In consumer code, we can always import roboticslab_yarp_devices as rlyd or import roboticslab_yarp_devices as rlyarpdevices

I agree, but PEP8 says:

Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

Source

Since 'discouraged' does not mean 'forbidden', I guess we could use roboticslab_yarp_devices as package name.

See #174. We should now be safe to remove that Python version constraint from the CMake code thanks to #195.

That's what I thought, but I wasn't 100% sure.

PeterBowman commented 5 years ago

We should now be safe to remove that Python version constraint from the CMake code thanks to #195.

Done at https://github.com/roboticslab-uc3m/yarp-devices/commit/ff690808731d3c75cd598f9a7179030b2df768f5. Please check https://github.com/roboticslab-uc3m/yarp-devices/commit/0c4d8405d5b23a343850078328612f26620bcefe, I had to change this path in order to run the setup script.

jgvictores commented 5 years ago

As commented in person, +1 to roboticslab_yarp_devices (and leave from roboticslab import yarpdevices as future work at org level)

PeterBowman commented 5 years ago

As commented in person, +1 to roboticslab_yarp_devices (and leave from roboticslab import yarpdevices as future work at org level)

Shall we proceed?

David-Estevez commented 5 years ago

:+1:

jgvictores commented 5 years ago

:+1:

PeterBowman commented 5 years ago

Hmmmm.

yarp-devices is NOT the package name, that would be GrabberControls2Gui (the packages parameter to setup()). If I keep that name='yarp-devices' parameter as-is, setuptools creates a /usr/local/lib/python3.5/dist-packages/yarp_devices-0.0.4-py3.5.egg directory. Then, I can import GrabberControls2Gui, but import yarp_devices.GrabberControls2Gui does not work. It doesn't either when I change it to name='rl_yarp_devices', only the directory name changes.

From https://packaging.python.org/tutorials/packaging-projects/#creating-setup-py:

name is the distribution name of your package. This can be any name as long as only contains letters, numbers, _ , and -. It also must not already taken on pypi.org. Be sure to update this with your username, as this ensures you won’t run into any name collisions when you upload the package.

packages is a list of all Python import packages that should be included in the distribution package. Instead of listing each package manually, we can use find_packages() to automatically discover all packages and subpackages.

Looks like yarp_devices is not taken on pypi.org (https://pypi.org/search/?q=yarp_devices). BTW: https://pypi.org/project/yarp/.

@David-Estevez @jgvictores shall we proceed with the rename to, say, roboticslab-yarp-devices? Or perhaps roboticslab-uc3m-yarp-devices to stay unique?

Addendum: the setup script currently fails in our Travis builds. Solution: add python3-setuptools package to the apt addon and add sudo to the make install and make uninstall commands. I'm not sure about this, some effort was made recently to avoid sudo. Let's note this down and revisit in the future.

David-Estevez commented 5 years ago

yarp-devices should be the current distribution package, which should include several packages. If it does not work like that, could be due to not having the typical Python package folder structure (a root folder named yarp_devices containing all the different modules. I'm not completely sure about this, but that's how I organized other python projects (with the same name/packages method) and it worked for me.

About the name, I would use roboticslab-uc3m-yarp-devices and I wouldn't worry too much about pypi.org, as pip allows installing a package using a git repo directly (see, for instance: https://medium.freecodecamp.org/how-to-use-github-as-a-pypi-server-1c3b0d07db2).

David-Estevez commented 5 years ago

I have found this post in Stack Overflow where they suggest using a single folder as the root of the package.

As spoken with @PeterBowman today, the solution might involve either copying modules to a temporal folder via CMake for installation or modifying the repo structure to have all python-related code in a package-friendly way.

Bonus: Not directly related, but useful if we want to make meta-packages (textiles, roboticslab-uc3m) in the future: https://packaging.python.org/guides/packaging-namespace-packages/

PeterBowman commented 1 year ago

I'm closing this as partially done, i.e. for now the CMake tweaks are good enough and we are not pushing further towards a solution for the naming problem. I have just added a reference to this issue in the setup file at https://github.com/roboticslab-uc3m/yarp-devices/commit/148ae01cf4a04ad3f3d88b676a45682746920619.

(Psst, in case you ever need help with Python stuff, I'm totally positive that @imontesino will know the answer!)