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 (Some questions about the solver and *.slvs format) #292

Open
KmolYuan opened this issue Sep 14, 2017 · 27 comments

Comments

@KmolYuan
Copy link

Hello. I made a Python 3 wrapper for the solver,

This project is using newest source code and compile without CMake. It can made slvs.h be import then used simply in a Python script (shown as PyDemo).

If allowed, I would like to add it to the "solver" branch.

On the other hand, I want external programs can read and write *.slvs format, with fully Link / Assemble support, then solvespace files can be generated from C or Python.

Best regard.

@KmolYuan KmolYuan changed the title Some question about the solver and *.slvs format Some questions about the solver and *.slvs format Sep 14, 2017
@ghost
Copy link

ghost commented Sep 22, 2017

For the record, soon it could be used for implement SolveSpace solver in Blender 2.7x or later

@whitequark
Copy link
Contributor

@KmolYuan The "solver" branch exists because of copyright reasons, not for technical ones. If you are aiming for integrating your Python bindings, that should be done in the "master" branch, where active development happens. Are you fine with that?

@KmolYuan
Copy link
Author

Sure!

If it can be integrated into the CMake build process it would be better.


By the way, the file file.cpp is using information from SK (Sketch class) of SolveSpaceUI class, It seems to be unrelated to solver parts.

Is possible to separate SK and UI parts, or just using SolveSpaceUI directly to write a *.slvs format?

@whitequark
Copy link
Contributor

OK. I will ask you to sign a CLA very soon and then I will be able to integrate your code. I will handle CMake and basic testing for you.

Right now the solver that is exposed via the external interface can only solve for one group, and files have many. So, the exposed solver cannot interact with files. I'm not sure what's the best way to fix this, I've not really considered the binding evolution much.

@KmolYuan
Copy link
Author

For the file format section, maybe there should be a new external interface with the internal data format.

Because I want to put the results of the external design process into Solvespace. There would be used as auto-assemble function.

@whitequark
Copy link
Contributor

For the file format section, maybe there should be a new external interface with the internal data format.

Unfortunately the way code is currently structured, the internal C++ interface does not check many invariants, so actually exposing it as-is in Python would cause a hard to debug crash in case of misuse. Of course we want to fix that but it is by no means quick.

The UI is not scriptable either.

@whitequark
Copy link
Contributor

Thank you for your contribution, and welcome. The SolveSpace project has a Contributor License Agreement; in order for me to be able to merge your changes, please sign it at https://cla-assistant.io/solvespace/solvespace. It will take only a small amount of your time.

@whitequark
Copy link
Contributor

@KmolYuan Are you still interested in integrating this?

@KmolYuan
Copy link
Author

KmolYuan commented May 24, 2019

Python-Solvespace project has been updated a new wrapper mechanism to Cython instead of SWIG at April 2019.

The old SWIG wrapper is terminated at KmolYuan/python-solvespace@24cfed4. They are not existed in the current version.

Wrapper source code:

  • Cython/slvs.pxd
  • Cython/slvs.pyx

Compile settings:

  • Makefile
  • requirements.txt (Python dependence)
  • setup.py
  • .travis.yml
  • appveyor.yml
  • platform/patch.diff (for Python compiler option update)

For development:

  • __init__.py (module expose for Python code)
  • slvs.pyi (stub file)
  • test_slvs.py (unit test)
  • example/*.slvs (cases in unit test)

The makefile is just used to call "python setup.py build_ext -j0 --inplace" and clean them up (non-essential). In the current test, Python-Solvespace can be coexist with Solvespace.

To merge them, there must be some work here:

Directory is as follows:

/solvespace
    /cython
        /python_solvespace
            __init__.py
            slvs.pyi
            slvs.pxd
            slvs.pyx
        /platform
            patch.diff
        /tests
            __init__.py
            test_slvs.py
        setup.py
    ...

@whitequark
Copy link
Contributor

Looks reasonable.

@KmolYuan
Copy link
Author

KmolYuan commented May 28, 2019

There are two different ways to add this solver wrapper.

Let solvespace as submodule in python-solvespace; or integrate the wrapper into the branch (or maybe new branch) of solvespace, then python-solvespace might be deprecated.

I think the former will be more convenience to publish the wrapper to PyPI with CI service. The submodule can follow master branch if Solvespace updated. I can do some testing first.

If the latter one, the wrapper packages still can released at a forked repository.

@whitequark Which way does this community prefer?

@whitequark
Copy link
Contributor

So, there's one thing to discuss first. SolveSpace uses the GPLv3+ license and has a CLA. Are you willing to relicense the wrapper as GPLv3+ and sign the CLA? If no, the bindings unfortunately have to be separate. If yes, then I propose putting the bindings in the same repository, so that they will always be tested with the most recent SolveSpace version, and distributed together.

@KmolYuan
Copy link
Author

KmolYuan commented May 28, 2019

OK, I will change the license of python-solvespace to GPLv3+.

I will fork this repository and do it at python branch as above plan.

@KmolYuan
Copy link
Author

KmolYuan commented Jun 6, 2019

The PyPI releases require user account and password from CI serves. In common way, the user name and password are stored in TWINE_USERNAME and TWINE_PASSWORD environment variables of Twine.

Travis CI (Ubuntu):

matrix:
    include:

        - os: linux
          sudo: required
          python: "3.6"
          dist: xenial
          deploy: &deploy-options
              provider: pypi
              user: $TWINE_USERNAME
              password: $TWINE_PASSWORD
              skip_cleanup: true
              skip_existing: true
              on:
                  tags: true

        - os: linux
          sudo: required
          python: "3.7"
          dist: xenial
          deploy:
              <<: *deploy-options

Travis CI (Mac OS):

after_success:
    - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then
        if [ -n "$TRAVIS_TAG" ]; then
            python3 -m pip install twine --user;
            python3 setup.py sdist bdist_wheel;
            python3 -m twine upload dist/*.whl --skip-existing;
        fi
        fi

AppVeyor (Windows):

artifacts:
    - path: dist\*.whl

deploy_script:
    - IF "%APPVEYOR_REPO_TAG%"=="true" (pip install twine)
         ELSE (echo deployment has been skipped as environment variable has not matched)
    - IF "%APPVEYOR_REPO_TAG%"=="true" (twine upload dist\*.whl --skip-existing)

Here are some things to do:

  • Change path of package.
  • PyPI account of solvespace (and contributors).
  • When to release? (by tags?)

@whitequark
Copy link
Contributor

@KmolYuan Thanks for preparing this. I'll look into it closer shortly.

@KmolYuan
Copy link
Author

Hello! I have published the wrapper named as python-solvespace on PyPI (under GPLv3+).

pip install python-solvespace

The CI / CD configurations are modified with "matrix" mechanism (Travis / AppVeyor). And I keep the old deployments on solvespace/solvespace, PyPI deployments on KmolYuan/solvespace.

I hope you guys can check them out if I do something wrong. Then I will open a pull request.

The distributions are supported with Windows (3.6, 3.7) and mac OS (3.6, 3.7, 3.8-dev), Linux user needs to build from source.

The package version 3.0.0 is depend on C++ kernel just like PyQt, the major and minor version are same, the micro version will changed with wrapper updating.

@ghost
Copy link

ghost commented Sep 25, 2019

Hello! I have published the wrapper named as python-solvespace on PyPI

Awesome!

But check "Project description" on PyPI website - seems like its markup is broken! (does PyPI support Markdown markup?)

@KmolYuan
Copy link
Author

KmolYuan commented Sep 25, 2019

@Symbian9 Oh, that's an upstream issue that PyPI can't update description unless release a new version.
(I forgot to set text/Markdown first time.)

Maybe I can publish 3.0.0.post0 to fix this.

@KmolYuan
Copy link
Author

KmolYuan commented Sep 26, 2019

Oh! I made a big mistake with Linux sources.

The upper directories and files needs to move into python_solvespace folder, otherwise they will excluded by sdist (packing sources) command. I will fix them in a while.


OK, I solved it by copy ../src and ../include to python_solvespace temporarily before building the libraries or packing the sources.

https://github.com/KmolYuan/solvespace/blob/python/cython/setup.py#L99-L112

@igorkb
Copy link

igorkb commented Oct 21, 2020

I am sorry but it's not clear. @KmolYuan, will you sign the CLA and merge the wrapper?

@KmolYuan
Copy link
Author

@igorkb Yes, I have signed. Python wrapper is at a stable version 3.0.3.post1 currently.

I can open a new PR if there comes another release. (maybe after a long time maintenance)

#493

@ruevs ruevs changed the title Some questions about the solver and *.slvs format Python Wrapper of the Solver (Some questions about the solver and *.slvs format) Oct 21, 2020
@ruevs
Copy link
Member

ruevs commented Oct 28, 2020

Hi @KmolYuan have you noticed that one more person seems to have started/done a Python wrapper here?
https://github.com/realthunder/solvespace/commits/python
It is quite out of date with master and I have not looked at the code (I do not know Python) but perhaps you and @realthunder could join forces. Or you could look at his for for useful pieces/ideas.

@KmolYuan
Copy link
Author

@ruevs Well, that wrapper is made by SWIG, which needs to write an interface file and compile it together.
This version uses CMake to complete the compilation process.

My early attempts did the same.
But because it is not easy to maintain the Python port, this solution was abandoned.
The advantage of Cython is that the syntax is similar to Python, and it is easier to expand Python's functions (such as docstring).

The tricky thing is Cython sources can not compile directly with CMake. Maybe something like scikit-build?

@realthunder
Copy link

In order to support assembly, the libslvs API needs to be extended. You can reference my patch here. Although it is based on an older version, it should still be relevant.

Some other useful patches, which may or may not be relevant any more,

  • Allow raising exception instead of abort on invalid input, (realthunder@b7c390d)
  • Expr memory optimization, as the original implementation for Linux is very inefficient (realthunder@1a38896)

@vespakoen
Copy link
Contributor

@realthunder I think those changes your pointed out would be really good to have in SolveSpace!
Would you consider making seperate PR's for those?
If not, would you mind if I would bring them over?

Has anyone ever considered making a (C++) Class that contains the solver only?
I imagine that would reduce duplication when making multiple bindings, I got this feeling because embind and boost.python seems to be pretty simple when using a Class.

@realthunder
Copy link

@vespakoen I don't think I have time for making a PR here. You are of course welcome to use my code.

@ghost
Copy link

ghost commented Sep 18, 2021

Awesome News for Blender users!

Thanks to @hlorus, now there is "Geometry Sketcher" add-on for Blender 2.92+ (written on top of @realthunder's py_slvs Python module) for adding SolveSpace 2D geometry constraints into Blender!

N.B. This add-on implementation finally closes (partially) my old request to @KmolYuan:

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

No branches or pull requests

6 participants