-
Notifications
You must be signed in to change notification settings - Fork 185
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
WIP: Improved Python bindings #3829
Conversation
This option generates a Python binding that executes the proxied C++ code faster. Adding the "olddeps" option generates a Python proxy with the correct method signatures so that calling code can treat it like Python, but the actual call is done with the proxied property. Note that another option, "builtin" was also tried, but this broke existing implementations so it was not suitable. For a comparison of the different methods see: https://www.swig.org/Doc4.2/Python.html#Python_optimization
Unlike the verovio.i file, where the type hints were in the proxy, but no other "direct" methods were typed, the .pyi file contains type hints for all the methods in the Python Verovio binding. The py.typed file signals that this package is typed. See: https://peps.python.org/pep-0561/
- Added a few methods to ignore that were anyway broken in the Python package and that used ostream arguments (difficult to know what type of data Python expects here) - The first argument for class methods is, by convention, 'self'. Modified this since the .pyi file used self for the "automatically" proxied code, and "toolkit" for the ones from the .i file. - Made "xml_id" consistent with "xmlId" arguments in the automatically-proxied code. - Added a proxy for "ValidatePAEFile" that turns the return string into a Python dictionary (same as "ValidatePAE")
WIP: Still needs a few things |
Maybe then mark it as draft? |
Hm, now why didn't I think of that? ;-D |
As far as we know, this is no longer necessary?
OK, ready for review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
def initClock(self) -> None: ... | ||
def resetClock(self) -> None: ... | ||
def getRuntimeInSeconds(self) -> float: ... | ||
def logRuntime(self) -> None: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular logic to the order? Alphabetical sorting would make hand-editing much easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the order that's in toolkit.h, which is in semi-alphabetical order.
But the ones at the end are kinda questionable, since these are annotated with @nodocs
and don't appear in the Verovio book, so they're not technically "public."
Updates the Python bindings with a few new capabilities: