Build and distribute Python pip package of verovio

[apacha]

It would be fantastic if the CI would build the verovio wheels for all different platforms and distributes them via pip, so Python developers can just say 'pip install verovio'.

For example, for building the application on Mac is a bit tricky, as you need to install several dependencies. I also ran into another problem that I was luckily able to solve by calling export ARCHFLAGS="-arch x86_64" before running the build.

One last thing that I would like to point out: The pip package verovio isn't taken yet, so it would make sense to publish the official version of verovio on it, before someone else might grab that name.

[rettinghaus]

@musicEnfanthen what do you think?

[musicEnfanthen]

Yes, this could be a valuable asset. (But it is not on me to decide, so let's ask @lpugin ;)

There are two different build scenarios:

1. Build the python toolkit on every commit and provide the (develop) build via action artifacts (cf. the JS toolkit here: /~https://github.com/rism-ch/verovio/actions/runs/341696268). This should be straightforward to add to the workflow, following the build instructions here: /~https://github.com/rism-ch/verovio/wiki/Building-instructions#building-the-python-toolkit . But maybe someone can point out the necessary steps if there is more to take care of.

2. For every stable release, publish verovio as a pip package (analogue to the npm package). That would require some registration process (@lpugin) and a detailed description of the necessary steps.

@apacha Could you point out what the problem was building the Python toolkit on MacOS? For the CI, it would not matter on which platform we build the toolkit, wouldn't it?

Another thought: The approach of 1) could be also used to provide the JAVA toolkit.

[lpugin]

I think it would make sense for stable releases but not on every commit. @apacha could you draft the pip package? That would be very helpful. We can then build it from the GH action and publish it from there.

(I just build the latest develop on MacOS 10.15.7 with the MacOSX10.15.sdk without problem, so it would be a good track down the issue your are reporting)

[musicEnfanthen]

Here is an example workflow how to publish python packages: https://docs.github.com/en/free-pro-team@latest/actions/guides/building-and-testing-python#publishing-to-package-registries

[lpugin]

Thanks! Quite useful. I think the tricky part will be to deal with the C++/swig build. I assume users will still need to compile them on their side. Am I wrong? Sorry, I have very little experience with Python modules.

[ahankinson]

I think users won't have to compile it if you build binaries for their platform. From https://pythonwheels.com:

"C extensions

PyPI currently allows uploading platform-specific wheels for Windows, macOS and Linux. It is useful to create wheels for these platforms, as it avoids the need for your users to compile the package when installing.

You will need to have access to the platform you are building for."

[lpugin]

For now moved to the wish-list /~https://github.com/rism-ch/verovio/wiki/Low-priority-wish-list
However, feel free to make a PR anytime!

[musicEnfanthen]

Preliminary cross-platform working draft: /~https://github.com/musicEnfanthen/verovio/actions/runs/417449074

Including data folder works for sdist run via MANIFEST.in
Same approach does not work for bdist_wheel, all possible options seem to rely on included data being in a subfolder

[musicEnfanthen]

Steps to get data resources involved (with credits to [1]):

• symlink from within bindings/python to ../../data folder: ln -s -v ../../data data (data folder must not exist beforehand inside bindings/python, -v displays created link)

• let python recognize the new created data folder (link) as a package by creating an empty __init__.py inside bindings/python/data

• under bindings/python create MANIFEST.in with: recursive-include data *

• adjust setup() in setup.py:

setup(name='verovio',
      version='3.1.0-dev',
      ...
      py_modules=["verovio"],

     ### add the following lines ###
      include_package_data=True,
      packages=['data']
      )

• fire up python setup.py sdist bdist_wheel

[1] https://stackoverflow.com/questions/61624018/include-extra-file-in-a-python-package-using-setuptools

[musicEnfanthen]

Not sure if data is at the right location now . Wheel checker complains about it being at the top level. Also complains about duplicated files: /~https://github.com/musicEnfanthen/verovio/runs/1544041739?check_suite_focus=true#step:16:7

The checks can be disabled if duplication and top level is what we want instead

[musicEnfanthen]

The next issue will be that TestPyPi (and probably PyPi, too) does not allow the upload of Linux specific wheels: pypi/legacy#120

One would have to rely on manylinux, so we are back at the first issue that get-git-commit.sh is not recognized inside manylinux's docker container. But maybe we can use a similar symlink approach?

[lpugin]

Not sure if data is at the right location now . Wheel checker complains about it being at the top level.

Where should it expected to be placed? We can move it there. We still need to change the path in the C++ code anyway.

Also complains about duplicated files: /~https://github.com/musicEnfanthen/verovio/runs/1544041739?check_suite_focus=true#step:16:7

The duplicated files warning should be disabled because we cannot change this. The files are expected to be the same (content and name).

[lpugin]

The next issue will be that TestPyPi (and probably PyPi, too) does not allow the upload of Linux specific wheels: pypa/pypi-legacy#120

One would have to rely on manylinux, so we are back at the first issue that get-git-commit.sh is not recognized inside manylinux's docker container. But maybe we can use a similar symlink approach?

That's quite annoying... I don't think symlink will work, unless manylinux takes care of resolving / updating the links when copying stuff to the docker container. I doubt it.

One way forward would be to have a script to copy everything we need into a directory and run setup from there. But basically we would more or less copy everything from the parent. Not very elegant. Alternatively we could move setup.py to the root directory. That would be quite a big change but maybe in the end the best approach. Any thoughts?

[rettinghaus]

Having setup.py in root doesn't hurt, does it? If it helps, this seems like the way to go.

[musicEnfanthen]

Yes, that is a great idea, since we can move it to the top level inside the action! Let's try

[musicEnfanthen]

Where should it expected to be placed? We can move it there. We still need to change the path in the C++ code anyway.

I have no idea what the correct location would be. Wheel checker has some restrictions for folder names at the top level including data, see here W005: https://pypi.org/project/check-wheel-contents/ Will try to put it under src as mentioned there. [But in principle, as along as the wheel is working in the end, I wouldn't care too much about this.]

The duplicated files warning should be disabled because we cannot change this. The files are expected to be the same (content and name).

Ok, that's fine. Will disable then.

[lpugin]

This should also make it possible to use cibuildwheel, which could also make it easier. @musicEnfanthen are you giving a try? I can then look at the resource path issue.

(Maybe one thing to look at is to have setup.py outputting things in one directory that we can add to gitignore, and to not many files and directories in ./, but this is a minor issue.)

[musicEnfanthen]

@lpugin Could you explain briefly the following construction, especially why the cd .. is needed? Is it the same like ../../tools...?

 generate the git commit include file
os.system("cd ..;../tools/get_git_commit.sh")

[lpugin]

The script needs to run from on level down the ./. With the python setup being in ./bindings/python is needs to move one level up. If setup is moved to ./ you would need to do os.system("cd tools;./get_git_commit.sh") (I think)

[musicEnfanthen]

Ok thanks. The new path seems to work.

[samuelbradshaw]

I was excited to see Verovio available via pip install, but I couldn't get it to work (module 'verovio' has no attribute 'toolkit'), which led me here, and I see now that this is still a work in progress. Is making Verovio available as a Python pip package still being actively worked on, or is it on a back burner?

[apacha]

It is a work-in-progress (that's why this ticket is still open), though you can already install it from the test-pypi site, e.g., with pip install -i https://test.pypi.org/simple/ --pre verovio. This one should already work.

[lpugin]

The latest release (3.1.0) is now available in the official index and can be installed with pip install verovio. The index includes some binaries for Linux, Windows and MacOS for various versions of Python. There is also a source distribution that should build automatically upon installation for cases where the binary is not available.

Many thanks to @musicEnfanthen and @alastair for their great help and the huge amount of time they invested into this!

[samuelbradshaw]

Awesome, thank you!