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

Python wrapper of the solver #493

Open
wants to merge 141 commits into
base: master
Choose a base branch
from
Open

Conversation

KmolYuan
Copy link

#292 The first pull request of Python wrapper branch.

New folder:

  • cython

Modified files:

  • .gitignore: Ignore the project settings of JetBrains IDE.
  • .travis.yml and appveyor.yml: Deployment of PyPI.
  • src/platform/platform.cpp: Add a type cast for MinGW and newer version compiler.

@vespakoen
Copy link
Contributor

Hello Yuan Chang, thanks a lot for this work!

I'd really like to merge this into SolveSpace, so we can have a "officially supported" SolveSpace PyPi package.

It would have been nice if this used the project's CMake, but since it is currently very hard to build the "solver only" with CMake, I can totally see why that has been avoided.
This might be something we can improve in the future, but for now, your setup.py seems to work very well.

One suggestion I have is to use "cibuildwheel" to compile the packages.
This uses the manylinux docker images that have an old glibc version, to ensure compatibility with older linux versions.
I have added it in a fork, and it seems to work pretty well, see: https://github.com/vespakoen/solvespace/tree/python

@jwesthues Can you create an account on PyPi (https://pypi.org/account/register/) and on Test PyPi (https://test.pypi.org/account/register/) and add the API keys to the CI secrets? (as PYPI_API_TOKEN and TEST_PYPI_API_TOKEN)

@ruevs @phkahler are you ok to merge this?

@jwesthues
Copy link
Member

Very cool, thanks. I'll await @phkahler's thoughts on merging, and create those accounts assuming it all makes sense to him.

@phkahler
Copy link
Member

phkahler commented Jan 7, 2023

@vespakoen @jwesthues I've let it sit because I don't want the maintenance burden. If someone else is willing to deal with this I'm fine with merging it - assuming it makes sense. I've looked into the details of this exactly zero minutes ;-)

@vespakoen
Copy link
Contributor

To me this looks like the most up-to-date, minimal implementation for a python wrapper of the solver.

It is also a part of SolveSpace that is very loved, and I think it would be good to have it in the official repository.

I am also fine spending some time on it to make sure it works and keeps working in the future.

@phkahler
Copy link
Member

phkahler commented Jan 9, 2023

@vespakoen rebase and merge it and you have the job ;-)

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

Successfully merging this pull request may close these issues.

None yet

10 participants