-
Notifications
You must be signed in to change notification settings - Fork 172
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
Hydrogen python bindings #1313
base: development
Are you sure you want to change the base?
Hydrogen python bindings #1313
Conversation
ef3ebfd
to
33d989b
Compare
0bc3f14
to
7dcd01d
Compare
Hey @charbeljc, We finally managed to talk about this PR internally. Sorry for the delay. First of all: Do you have an use case your are aiming for or know other people that have? Also, what exactly will these changes encompass when everything is done? We also wondered where would be the best place to put this code. Inside the main repo does not appear to be a fitting place. After all, we are still in the process of making improvements and modernization of the core (some parts are more than 20 years old. You already have seen the mess 😄 ) and try to keep You are also working on a custom Python binding library, right? Is it because |
Hi @theGreatWhiteShark no worries about the delay, I understand that introducing python bindings like this needs careful overview and understanding of the move. I'll try to clarify my goals.
Ok, what's the purpose of those python bindings, after all ? To make it short, QA, refactoring support, API stability tracking, lower the entry point to the code base. It might give some ideas later to some people (including me) to hack something with those bindings, but the main point is to allow us to easily write more unit tests for all aspects of hydrogen core with the ease of use of the pytest framework. Right now, I did back off from my first draft, where the equivalent of If you look at my patch, You'll see that if you take apart the parts that are included in my work on H2Core::Object templatification, This patch is pretty additive and for now optional. Once we are done with this PR on H2Core::Object work, I will rebase this one.
Honestly Hydrogen code base is not that a mess, I've seen much worse in my (too long) career :-) But that's exactly the purpose of introducing python bindings as an internal quality tool. Having those bindings as an internal consumer of Hydrogen API, used as part of our build process with an easy to augment pytest unit tests collection will actually help us refactoring Hydrogen without breaking it.
I was maybe a bit imprecise on this part. I'm working on a custom Python bindings generator tool, targeting stock pybind11, headers only, C++ python binding framework, which is one of the most beautiful, robust and battle field proved way of making python bindings for C++ code (see this for instance). I just didn't found what I wanted in terms of code generation, so I ended up hacking my (for now) quick and dirty one in python, leveraging clang python bindings to parse hydrogen's headers. So to be clear, maintaining those python bindings outside Hydrogen would defeat the main goal as an internal QA tool. Besides, If I can make guaranties on my ability to maintain just the binding generator part, I can't give you any assurance that I could have enough time to maintain the actual python bindings as a standalone package, with the burden of publishing them on pypi and so on. As for having an libhydrogen-gui-1.1.0.so, that's also part of the plan, yes, although I must take the time to experiment more on the binding side, because, when it comes to Qt heavy code base, making use of shiboken would be maybe more natural. Anyway, i will extract a PR with just the libhydrogen-gui.so part, It won't hurt anyway to have it merged quick. To conclude, having python bindings as integral part of Hydrogen to support unit testing is the way to go, as I can assert from my recent patches. (I have on my machine some pretty impressive demos regarding automated gui testing of pan and velocity envelope editing, not ready for prime time, alas). So the plan would be. Heat our cake via pytest in the first place without bothering to publish those bindings to pypi. Then, if there is interest from someone to actually use them to make noise in an innovative way, reconsider our position and take the burden to actually publish them. Regards, |
Ah, I see. We mistook the testing use case for an arbitrary example instead for the main purpose. So, |
There is, in fact a great ecosystem around I did have a look at Hydrogen, maybe 10 or 15 years ago, and, as an amateur Batucada player I love it much because it actually helped me understand many things in percussion. We have only 53 tests under the I'm currently extending my python binding generator so that it also generate (So, to conclude, yes, I'm a QA boy, I guess, but a serious one, bringing actual tooling, and not using excel or making slides :-) ) |
Ah, I see, this makes a bit more sense of the motivation. Thanks for clarifying! I think the lack of existing tests speaks more to the nature of the project rather than the available testing infrastructure, namely:
If Pytest can help with any of the things in bucket number 2, I would like to hear about it! :) |
Granted, writing tests is not as fun as adding features... That's a reason why if we can bring fun in hydrogen testing with python bindings, it should be a good thing.
Yes, I know, audio and realtime properties can be hard to test, especially if your code base is not prepared for this kind of kung-fu. For the GUI aspect, I think we could leverage pytest-qt at least to unit test our dedicated knob widgets and likes. I did some extensive work on Sample/Instrument Editor, and my python bindings actually helped me in the process (see this for instance). But let me insist. The magic don't comes from the |
Second take at my experiments with pybind11. Most of libhydrogen-core classes are accessible from python. Tested with python3 only.
In a virtualenv with python3
Be ready for segfaults and coredumps. Regards.