Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TODO] Full python integration #207

Open
2 tasks
syoyo opened this issue May 13, 2019 · 19 comments
Open
2 tasks

[TODO] Full python integration #207

syoyo opened this issue May 13, 2019 · 19 comments

Comments

@syoyo
Copy link
Collaborator

syoyo commented May 13, 2019

Finish writing full python binding using pybind11(python/binding.cc)

Status

  • Implement binding to tinyobj struct/class(e.g. material_t)
    • Mostly done
  • Writer
  • .mtl reader/writer

Related to #148

@paulmelnikow
Copy link
Contributor

Hi Syoyo! I maintain a Python library called Lace and also use Trimesh. I'm evaluating tinyobjloader as a replacement loader that could work with both projects. (Lace has its own loader using C++/Boost/NumPy, and Trimesh has one in pure Python.) I'm planning to write tinyobjloader integrations for Lace and Trimesh (which I'd then add to / offer to add to the libraries themselves).

I'd also like to help smooth out the Python interop so it's easy for Lace + Trimesh users to consume this library. So far I've run into two issues:

  1. The source distribution doesn't install cleanly (see below) because it needs the pybind11 headers. I can see if I can come up with a solution for that.
  2. The version shipped to PyPI includes a source distribution but not a wheel, at least for my OS (Mac OS). Shipping a wheel makes the library fast and easy to install.

Regarding the wheels, out of curiosity how are you handling releasing to PyPI? Do you release from your dev machine or from CI? I think a wheel needs to be built on the target platform, though I could see what kind of tools are available with the CIs you're already using, and what I might be able to put together using Docker.

Thanks again for your work on this library. I'm excited at the prospect of using it in Lace + Trimesh.

Collecting tinyobjloader
  Using cached https://files.pythonhosted.org/packages/c0/ee/f447ed517ff301034b30a1f28fc76365f20bbb70480ca3b7d773803306b1/tinyobjloader-0.1.tar.gz
Building wheels for collected packages: tinyobjloader
  Building wheel for tinyobjloader (setup.py) ... error
  Complete output from command /usr/local/opt/python/bin/python3.7 -u -c "import setuptools, tokenize;__file__='/private/var/folders/m2/3rwkhb908xnfvd0059_t24yh0000gq/T/pip-install-zcvt76lh/tinyobjloader/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /private/var/folders/m2/3rwkhb908xnfvd0059_t24yh0000gq/T/pip-wheel-7gm6di_t --python-tag cp37:
  running bdist_wheel
  running build
  running build_ext
  building 'tinyobjloader' extension
  creating build
  creating build/temp.macosx-10.14-x86_64-3.7
  clang -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers -I../ -I../pybind11/include -I/usr/local/include -I/usr/local/opt/openssl/include -I/usr/local/opt/sqlite/include -I/usr/local/Cellar/python/3.7.3/Frameworks/Python.framework/Versions/3.7/include/python3.7m -c bindings.cc -o build/temp.macosx-10.14-x86_64-3.7/bindings.o -std=c++11
  bindings.cc:1:10: fatal error: 'pybind11/pybind11.h' file not found
  #include "pybind11/pybind11.h"
           ^~~~~~~~~~~~~~~~~~~~~
  1 error generated.
  error: command 'clang' failed with exit status 1
</details>

@syoyo
Copy link
Collaborator Author

syoyo commented Nov 4, 2019

PyPI build follows standard procedure from this: https://packaging.python.org/tutorials/packaging-projects/

The source distribution doesn't install cleanly

You may need to add pybind11 files to setup.py(but I don't know how to do it). PR is always welcome.

@paulmelnikow
Copy link
Contributor

You may need to add pybind11 files to setup.py(but I don't know how to do it). PR is always welcome.

Okay, I will look into that!

Regarding wheels, I understand what's happening now. There is a Linux wheel but not for Mac or Windows:

https://pypi.org/simple/tinyobjloader/

The wheels for other platforms need to be built on the other platforms, which is a challenge. I found this too. which could help:

https://github.com/joerick/cibuildwheel

The tool is new, but it's used by big projects like websockets and Twisted.

Would you be interested in running something like that? I could help set it up.

@paulmelnikow
Copy link
Contributor

paulmelnikow commented Nov 4, 2019

Regarding the build issue for the source distribution, one issue is that the pybind11 sources are not shipped with the source distribution. This is the full contents:

.
├── tinyobjloader-0.1
│   ├── PKG-INFO
│   ├── README.md
│   ├── bindings.cc
│   ├── setup.cfg
│   ├── setup.py
│   ├── tiny_obj_loader.cc
│   └── tinyobjloader.egg-info
│       ├── PKG-INFO
│       ├── SOURCES.txt
│       ├── dependency_links.txt
│       └── top_level.txt
└── tinyobjloader-0.1.tar.gz

It seems like the more common way to solve this is declaring an install dependency on pybind11 (as in the setup.py for mplcairo). Added: Here is another, much simpler example: https://github.com/pybind/python_example/blob/master/setup.py

If you prefer to keep the subtree approach, it's probably possible to vendor in pybind11 with your source distribution. However, it has a different license, so that might be confusing.

@paulmelnikow
Copy link
Contributor

I have a version which installs pybind11 at build time. It seems to be working well. master...metabolize:fix-python-sdist

@syoyo
Copy link
Collaborator Author

syoyo commented Nov 5, 2019

I have a version which installs pybind11 at build time. It seems to be working well.

Awesome! Could you please send this changes as an PR?

https://github.com/matplotlib/mplcairo/blob/f5bafd038ea6337ee2b9a21f7a2b279598ea9dc5/setup.py#L277

I didn't realized that such a feature in setuptools. Good to know!

@paulmelnikow
Copy link
Contributor

I will! I’m just digging in now to one last complication which is making pybind11 available at build time without unnecessarily installing it at runtime.

I also got cibuildwheel running on Azure Pipelines and used it to publish some experimental wheels. Python packaging is complicated and can be opaque, and I’m really impressed with the documentation for that tool, and how easy it makes the process.

@paulmelnikow
Copy link
Contributor

Hi @syoyo! Could you tag a new version and release it to PyPI, along with the wheels that get generated?

Also, I'll put an offer out there. I noticed some discussion about releases in #212, and would be happy to maintain the releases for this project, according to Semantic Versioning. I maintain a bunch of reputable projects, most notably https://shields.io/ and https://github.com/nock/nock/. I'm paulmelnikow on PyPI. Let me know if you'd like my help with this!

@syoyo
Copy link
Collaborator Author

syoyo commented Feb 17, 2020

@paulmelnikow I have added a tag for recent master branch.

would be happy to maintain the releases for this project, according to Semantic Versioning

Awesome! How can I delegate permissions of release versioning, uploading PyPI, etc to you?

I found this on github

https://github.blog/2019-10-03-delegate-responsibilities-with-expanded-repository-permissions/

@paulmelnikow
Copy link
Contributor

paulmelnikow commented Feb 17, 2020

Cool! You can give me write access to the repo here:

https://github.com/syoyo/tinyobjloader/settings/access

Write access isn't restricted to tagging releases, though if you'd prefer, I can refrain from using the access to do anything else. Probably it will be necessary to push commits to update version numbers and changelogs, as well, though I can definitely run those through pull requests.

You can add me to the PyPI project here:

https://pypi.org/manage/project/tinyobjloader/collaboration/

In the future it might be useful to move the project to a GitHub organization, which would allow delegating more access, such as managing connected apps, and using those expanded repository permissions you linked to. Though, that doesn't seem necessary right now. This is a great first step!

@paulmelnikow
Copy link
Contributor

Maybe you should also give me access to publish to Conan? To minimize confusion, ideally all the channels should get a new release with the same version number at the same time.

@syoyo
Copy link
Collaborator Author

syoyo commented Feb 18, 2020

@paulmelnikow Thanks! I have created an organization: https://github.com/tinyobjloader

Migrating the repo will be soon.

Maybe you should also give me access to publish to Conan?

Github organitzation + proper permission setting will also enable accessing Conan to you?

@paulmelnikow
Copy link
Contributor

I’m not sure! I’ve never used Conan and I’m not familiar with how it works.

@syoyo
Copy link
Collaborator Author

syoyo commented Feb 19, 2020

@paulmelnikow OK. Now I have moved the repo to tinyobjloader organization and added write permission to members, so you'll now have an push/pull/etc access to tinyobjloader repo!

@paulmelnikow
Copy link
Contributor

Did you add me to PyPI too? Why don’t I try publishing the release you tagged to PyPI?

@syoyo
Copy link
Collaborator Author

syoyo commented Feb 19, 2020

Screen Shot 2020-02-20 at 1 16 48

@paulmelnikow Added as maintainer! If you need Owner rule, let me know

@paulmelnikow
Copy link
Contributor

Since cibuildwheel wasn't working on the 2.0.0rc4 rev, I'll tag rc5 and get this going.

@paulmelnikow
Copy link
Contributor

Could you set up Azure Pipelines and Travis on the new repo?

@syoyo
Copy link
Collaborator Author

syoyo commented Feb 19, 2020

Could you set up Azure Pipelines and Travis on the new repo?

Oh, I see. Will look into that soon(tomorrow or day after tomorrow)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants