3MFConsortium / lib3mf_python

Lib3MF PIP package repository for Python use
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

PyPI py-lib3mf package #1

Open jdegenstein opened 1 month ago

jdegenstein commented 1 month ago

Hello,

I am the maintainer of https://github.com/jdegenstein/py-lib3mf and the related https://pypistats.org/packages/py-lib3mf

I am interested in collaborating on a PyPI package to reduce user confusion. Please let me know how/if you would like to proceed.

vijaiaeroastro commented 1 month ago

Hi John,

I just saw your email. Happy to finally receive a response from you. We are interested in maintaining official Python bindings and would love to work together with you. Since version 2.3.1 is already released, we are packaging it here in this repository and deploying to PyPI.

Another key thing we are doing is avoiding the need to know the paths of DLLs or .so files. You can see in this repository that there is a file at https://github.com/3MFConsortium/lib3mf_python/blob/main/lib3mf/__init__.py which adds some syntactic sugar. If there is a real need for it, we are happy to add much more syntactic sugar to the wrapper to make it easier to work with the bindings.

In future releases, starting from 2.3.2, the lib3mf repository itself will produce Python wheels as artifacts, and this repository will simply be a place to trigger deployments to PyPI.

We have also set up a few Python examples now to stay on par with C++.

Feel free to suggest improvements or changes on any of this. If you have any suggestions on how you would like to work together, you can let me know privately in email too and I can connect you with the rest of the developers in the project.

jdegenstein commented 1 month ago

I have a number of recommendations for this package.

  1. Move to a pyproject.toml setup (without setup.py) -- this is how my repo above was configured and it is intended to be the standard for packaging of python projects moving forward.
  2. Currently both this repo and my repo release a package to PyPI that implies it is not platform specific. This is obviously not true since there are 3 bundled compiled libraries that AFAIK support only 4 total combinations of system + machine types. For instance, this package will likely install perfectly happily on a raspberry pi running Linux, and yet it will not operate since the so is x86_64 specific. What needs to change is that there should be platform-specific wheels that ideally ONLY bundle their system library. This should also clarify any linux compatibility issues with GLIBC version used to compile the so. I have been documenting investigation of how to solve this here https://github.com/jdegenstein/py-lib3mf/issues/1
  3. I am not sure what purpose the lib_path and related functions in your __init__.py file serves. My original version here https://github.com/jdegenstein/py-lib3mf/blob/d7b71b9746bc0cabc5d6f6ef399c0f28f239ef82/py_lib3mf/__init__.py does not include these features and appears to work perfectly in my testing. I believe there is some logic in Lib3MF.py to find the libs, and since they are bundled with the python wheel their locations should not be ambiguous (from the perspective of this package). I could be wrong about this, and perhaps our goals were different here.
  4. There is room for additional syntatic sugar in __init__.py but it should be taken in context with #2. In other words, system/machine type checking may become redundant from publishing the wheels separately (linux+x86_64, darwin+x86_64, darwin+arm64, windows+x86_64 -- which will be 3 total wheels given the universal2 dylib from lib3mf>=2.3.1).

I plan to reach out again by email as well.

vijaiaeroastro commented 1 month ago

Thank you so much for your response.

  1. I will switch to pyproject.toml in the next release (as 2.3.1 has already been published).
  2. We have had discussions regarding this. Originally, we only produced x64 wheels for three platforms. By sheer coincidence, I switched to a "universal" wheel and later discovered your work, which was a pleasant surprise. I chose this option (for the time being at least) to reduce the number of artifacts on GitHub. From the next lib3mf release, Python wheels will also be produced as artifacts, and having too many would make that list look awfully cluttered. We will definitely pursue this depending on demand.
  3. You are absolutely correct about this. I apologize for copy pasting some of the content in the previous message.The init method of the Wrapper class in Lib3MF.py does have the same logic, but it only checks the same folder, and one needs to explicitly pass the library path if it's elsewhere. Originally, we did not have the libraries and dll's laid out in the same directory level as Lib3MF.py, hence I implemented this. It may seem redundant now since I laid out the libraries in the same location as Lib3MF.py (just as you did). If we end up switching to a different directory structure in the future, I wanted to leave room for this.
  4. I agree. But once again, if there is demand, then we will do this. We are not entirely sure how many people are using it. There was not a rich set of Python examples either, so I wanted to do both at the same time before we made an announcement.

I once again thank you so much for your time, John. Feel free to reach out whenever. Once I have the workflow and PR setup in the official lib3mf repository, I will also include you in the email chain, and you can share your feedback.