[RFC] Support for model implementations written in Python #78
Conversation
This is a work in progress and is not ready to merge. See docs/models for documentation.
| @@ -0,0 +1,293 @@ | |||
| import concurrent.futures | |||
There was a problem hiding this comment.
pyotherside currently has no infrastructure to ship pure Python sublibraries to be available to library users.
import pyotherside provides a C library. Maybe we could make this code available under import pyotherside.model? This would need some work to implement, I think. If this sounds OK to you, shall I look into what it would take to make this available under such an interface?
|
|
||
| return constructor | ||
|
|
||
|
|
There was a problem hiding this comment.
Below is my testing before I added proper automated tests. I'll drop this.
| @@ -1,3 +1,4 @@ | |||
| CONFIG += debug | |||
| @@ -1,5 +1,5 @@ | |||
| isEmpty(PYTHON_CONFIG) { | |||
| PYTHON_CONFIG = python3-config | |||
| PYTHON_CONFIG = x86_64-linux-gnu-python3.4-dbg-config | |||
| } | ||
|
|
||
| PyMethodDef pyotherside_QPythonItemModelProxyType_methods[] = { | ||
| {"call_back_for_modification", (PyCFunction)call_back_for_modification, METH_O, |
There was a problem hiding this comment.
I've implemented this inside PythonItemModelProxy for now. But perhaps it's a more generic thing for a Python thread to want to do something in the UI thread, and thus it should be a generic method available through the pyotherside module?
| py.call('main.get_model', [], function(result) { | ||
| // this should change to: | ||
| // model.change_model(model_constructor); | ||
| // in future as a property is inappropriate |
There was a problem hiding this comment.
Is a property really inappropriate? I think it'd be fine as property.
|
|
||
| and then in main.py: | ||
|
|
||
| import pymodel |
There was a problem hiding this comment.
Can we name this pyotherside (there's already an import for that) and the class ListModel (as is already the case), so that it's pyotherside.ListModel()?
|
|
||
| import pymodel | ||
|
|
||
| def get_model(bridge): |
There was a problem hiding this comment.
Continuing from the example above, there's no "bridge" parameter supplied in the QML example code above.
|
|
||
| * I expect to rebase this branch before submission. | ||
|
|
||
| * There end up being three "faces" in Python. Suggestions for better names |
| speaks in QML about the model. | ||
|
|
||
| * I wonder if there will be a significant performance impact in putting the | ||
| model into Python. I guess I'll need to try it and see. |
There was a problem hiding this comment.
Yes, having some performance benchmarks (native Qt/QML ListModel vs. your Python model to see how much % slower the Python implementation is) would be good. Even better if those benchmarks are available as regression tests in the source tree :)
| as part of the pyotherside module import. This needs build system thought | ||
| since pyotherside currently doesn't ship any Python. | ||
|
|
||
| * I want to keep the use of pymodel.py optional, and maintain the API between |
There was a problem hiding this comment.
Yes, would be good if that is implemented in native code as part of the native pyotherside Python module.
| import io.thp.pyotherside 1.5 | ||
|
|
||
| Rectangle { | ||
| property bool sorted: false; |
There was a problem hiding this comment.
QML Style guidelines do suggest that no trailing semicolons are used here
| /** | ||
| * PyOtherSide: Asynchronous Python 3 Bindings for Qt 5 | ||
| * Copyright (c) 2011, 2013, 2014, Thomas Perl <[email protected]> | ||
| * Copyright (c) 2015, Robie Basak <[email protected]> |
| @@ -71,4 +72,5 @@ PyOtherSideExtensionPlugin::registerTypes(const char *uri) | |||
| qmlRegisterType<QPython15>(uri, 1, 5, PYOTHERSIDE_QPYTHON_NAME); | |||
| qmlRegisterType<PyGLArea>(uri, 1, 5, PYOTHERSIDE_QPYGLAREA_NAME); | |||
| qmlRegisterType<PyFbo>(uri, 1, 5, PYOTHERSIDE_PYFBO_NAME); | |||
| qmlRegisterType<QPythonItemModel>(uri, 1, 5, PYOTHERSIDE_QPYTHONITEMMODEL_NAME); | |||
|
@basak did you have time to look at my comments from January? |
|
On Sun, Jun 04, 2017 at 02:39:50AM -0700, Thomas Perl wrote:
@basak did you have time to look at my comments from January?
Sorry, not yet. I'm not sure I'm going to have much time to look at it
soon. I do still intend to get to it eventually.
|
|
Anyone working on this? :) |
|
Closing for now, since there hasn't really been any activity on this in years. |
This is an RFC for issue #52. It's not ready - there are a few things I want to fix up myself, but as requested here it is for easier inline comments. I'll add my own inline comments for discussion.