Skip to content
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

BUG: Fix python wrapping for classes with only default constructor #796

Closed
wants to merge 1 commit into from

Conversation

hina-shah
Copy link

This PR includes the following:

  • Fixes regular expression in ctkWrapPythonQt.py which parses the class constructors to populate the classes to be passed to PythonQT for wrapping. It was skipping the classes which only had default constructors (i.e. no constructors of the type className(QObject* ...). Also, the closing parenthesis was not identified. @jcfr: you might want to check the RE! I have compared the build outputs (by switching on verbose in the ctkMacroWrapPythonQt.cmake), and the following classes are added to Python wrapping now: ctkErrorLogLevel, ctkErrorLogQtMessageHandler, ctkErrorLogTerminalOutput, and ctkEventTranslatorPlayerWidget. ctkErrorLogAbstractMessageHandler is skipped because it has a pure virtual method, and not because the constructor type is wrong. The other classes are unaffected.

  • Adds a default constructor to ctkCheckableHeaderView class so that it will be included for Python wrapping. This is actually a hack. Some methods of the class are enabled for Python wrapping as well.

@pieper
Copy link
Member

pieper commented Apr 11, 2018

👍 This looks good to me, but let's hold off until @jcfr is back to look at it.

@hina-shah
Copy link
Author

@jcfr : When you get time, could you please look at this PR? Thanks! CircleCi failed on uploading the report at the end, but other than that all the tests were passed.

@jcfr
Copy link
Member

jcfr commented Apr 23, 2018 via email

@jamesobutler
Copy link
Contributor

Hi @jcfr, could you take a look at this again? Thanks!

@lassoan
Copy link
Member

lassoan commented Sep 23, 2018

@jcfr Please have a look at it whenever you have time. Thanks!

@lassoan
Copy link
Member

lassoan commented Apr 5, 2023

Superseded by #985 and #1078

@lassoan lassoan closed this Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants