-
Notifications
You must be signed in to change notification settings - Fork 335
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 modules improvements #1572
Python modules improvements #1572
Conversation
Add ability to provide add_copy_py_file_to_sandbox_py_module_command and similar functions with modules containing '.' (e.g. appleseed.studio)
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.
Thanks for improving our python packaging cmake macros.
I'd prefer not to add the extra appleseed directory inside appleseed.python.
Other than that, I have some small comments and it'd good to merge for me.
@@ -167,10 +167,5 @@ PythonInterpreter::PythonInterpreter() | |||
{ | |||
} | |||
|
|||
PythonInterpreter::~PythonInterpreter() |
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.
Can we leave the destructor, comment Py_Finalize() and add a comment to explain why we don't call it?
@@ -27,3 +27,4 @@ | |||
# | |||
|
|||
from _appleseedstudio import * | |||
import Qt |
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.
We should remove this line. Using any python Qt binding should be optional and if the user does not have one,
we should not error.
b6771c5
to
66fe751
Compare
Refactored of appleseed.python/CMakeLists.txt to update build process of Python bindings
Added Qt.py - library that wraps different Qt Python binding (PyQt, PySide and so on) and provides unified interface for them