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

Missing python wrapping of ctkCheckableHeaderView #793

Closed
jamesobutler opened this issue Apr 6, 2018 · 9 comments
Closed

Missing python wrapping of ctkCheckableHeaderView #793

jamesobutler opened this issue Apr 6, 2018 · 9 comments

Comments

@jamesobutler
Copy link
Contributor

While using Slicer I noticed that the python wrapping for the ctkCheckableHeaderView was missing.

There was ctkCheckableHeaderViewEventPlayer() and ctkCheckableHeaderViewEventTranslator(), but the main widget was missing.

@lassoan
Copy link
Member

lassoan commented Apr 6, 2018

Could you please send a pull request that fixes the problem?

@hina-shah
Copy link

The problem occurs because ctkCheckableHeaderView constructor does not have the desired signature (as specified by ctkWrapPythonQt.py).

Please advise about the desirable solution to this. Here are the options that I can think of:

  1. Adding another constructor to ctkCheckableHeaderView with default parameters and correct order needed for python wrapping.
  2. Changing ctkWrapPythonQt.py itself so that in future other cases might be handled correctly. (Although this seems to be a bit more difficult)

@lassoan
Copy link
Member

lassoan commented Apr 9, 2018

Adding a constructor that does not require any input arguments sounds like a simple solution.

@kerim371
Copy link
Contributor

kerim371 commented Jul 23, 2021

I encountered the same problem.

@lassoan It is a little problematic to add a constructor that doesn't require any input argument as ctkCheckableHeaderView(QWidget *parent = nullptr) inherits from QHeaderView(Qt::Orientation orientation, QWidget *parent = nullptr) and QHeaderView orientation must be set while one initializes the object (in constructor) as it doesn't have something like QHeaderView::setOrientation(Qt::Orientation orientation).

I solved this by creating two classes (one for horizontal header and another for vertical by analogy with QHBoxLayout and QVBoxLayout) that inherit from ctkCheckableHeaderView:

  • ctkCheckableHHeaderView(QWidget *parent = nullptr) : QHeaderView(Qt::Orientation::Horizontal, parent)
  • ctkCheckableVHeaderView(QWidget *parent = nullptr) : QHeaderView(Qt::Orientation::Vertical, parent)

This solution would mean that from C++ one is free to use any of 3 ctk header views while in python there are only 2 classes would be available.

Also there are at least two methods that ctkCheckableHeaderView adds to public but not to public slots (thus they are unavailable in python):

  • Qt::CheckState checkState(int section)const;
  • bool checkState(int section,Qt::CheckState& checkState )const;
    and probably:
  • ctkCheckableModelHelper* checkableModelHelper()const; (I don't know what it is yet)

I think we should move them to public slots section

If you agree or have some sugestions I could make a PR

@pieper
Copy link
Member

pieper commented Jul 23, 2021

Creating the extra classes sounds like a good solution to the constructor issue.

The other two you listed do look like they should be slots, but ctkCheckableModelHelper* checkableModelHelper()const; may be better as either a Q_PROPERTY or just a method decorated with Q_INVOKABLE so it will be python wrapped.

@lassoan
Copy link
Member

lassoan commented Jul 23, 2021

Do not create new classes if you just want to a make additional constructors accessible from Python! Just add the appropriate decorator methods. There are several examples for this in CTK.

Do not export checkableModelHelper, as it sounds like an internal helper class.

To make simple properties (int, string, string list, enum,...) accessible from Python, it is preferable to declare them as properties as @pieper suggested above.

@kerim371
Copy link
Contributor

Today I have achieved some success in understanding PythonQt decorator's staff (mostly from this article) and I think tomorrow I will try to implement it in ctkCheckableHeaderView PR

@pieper
Copy link
Member

pieper commented Jul 25, 2021

Thanks for posting the link to that article, somehow I never saw it before (wish I'd read it years ago!).

@jamesobutler
Copy link
Contributor Author

Closed by fa1732f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants