dfki-ric / pytransform3d

3D transformations for Python.
https://dfki-ric.github.io/pytransform3d/
Other
615 stars 63 forks source link

Split package into three ones: rotation, translation and transform_managers. #236

Open szobov opened 1 year ago

szobov commented 1 year ago

Hi there, Thanks a lot for this amazing project. It's great!

I want to bring it to more widespread adoption and adopt it for internal projects. But the showstopper I faced was that the project required some significant dependencies: "scipy", "lxml", "beautifulsoup4", which are used only in transform_manager's related code.

I think it can bring significant benefits if we split the project into three separated:

  1. transformations, with dependencies: numpy and soft dependency matplotlib
  2. rotations, with dependencies: numpy and soft dependency matplotlib
  3. transform_managers: with dependencies: transformations, rotations and all other dependencies.

It will enable users to install required packages selectively if they don't need to use transform_managers. If we agree on it, I can implement all required code changes. I am eager to hear your opinion on it.

Thanks in advance, Sergei

AlexanderFabisch commented 1 year ago

Hi @szobov ,

Thanks for your feedback!

You are right, there are too many non-optional dependencies right now. The code is written in a way that you should still be able to use most components without installing scipy or beautifulsoup4. A quick solution for the issue is to install it via pip with --no-deps and install numpy and matplotlib manually.

I'd like to avoid splitting the package into three separate packages. That would break a lot of existing code. I'd rather suggest making scipy and beautifulsoup4 optional dependencies and raise helfpful errors when an optional dependency is missing. I think the best solution would be to select an appropriate set of dependencies like this in pip:

pip install pytransform3d[transform_manager]
pip install pytransform3d[visualizer]
pip install pytransform3d[transform_manager,visualizer]

etc.

AlexanderFabisch commented 1 year ago

Here is the list of dependencies (including latest features that are under development):

required

plotting

plotting of meshes

visualizer

transform_manager

graph visualization

urdf

jacobians

uncertainty

documentation

test

szobov commented 1 year ago

Thanks for a quick reply @AlexanderFabisch ! Sure, we can do it this way. If we agree on it, I can try to implement those changes. What do you think?

AlexanderFabisch commented 1 year ago

Yes, that would be very helpful! I'd suggest to update the setup.py and documentation accordingly. Also maybe it is worth to take a look at how other Python packages manage their optional dependencies. pandas has lots of those for example. I'm happy to review any pull requests.

AlexanderFabisch commented 1 year ago

Release 3.0.0 is about to be released soon. Should this be part of it?

szobov commented 1 year ago

Release 3.0.0 is about to be released soon. Should this be part of it?

Hi @AlexanderFabisch , I'm sorry, but I got covid, and I don't think you should rely on me to wait for a release. Another option could be if you'll let me know the planned date so I can estimate whether I can be on time. I appreciate your understanding.

AlexanderFabisch commented 1 year ago

I won't release it before next week. Get well soon!

szobov commented 1 year ago

Hey @AlexanderFabisch I'm sorry for the delayed reply. I recently started working on the implementation. I decided on implementing a pandas approach, but with a bit of a change to decouple version setting. (I was not happy with writing package names in many places) I'll make a draft PR soon so that we can discuss it in review. Thanks for your patience.

AlexanderFabisch commented 1 year ago

No worries, there is no time pressure :)