Breaking QCheckbox stateChanged signal change in Qt6

[Tony Schaefer]

The following bug report has been submitted:
Platform:        Windows-10-10.0.19044
ChimeraX Version: 1.4.dev202203120056 (2022-03-12 00:56:56 UTC)
Description
Not sure if bug, but an annoying thing with Qt6. With QCheckBox's stateChanged signal, I used to be able to do something like this:

from Qt.QtCore import Qt
from Qt.QtWidgets import QCheckBox

def some_function(state):
    if state == Qt.Checked:
        (do something)
    elif state == Qt.Unchecked:
        (do something else)

box = QCheckBox()
box.stateChanged.connect(some_function)

Now with Qt6, state is an integer , and Qt.Checked and Qt.Unchecked are some CheckState thing. The CheckStates are never equal to the state from the signal, so some_function won't do anything. I seem to have to define it as:

def some_function(state):
    if state == 2:
        (do something)
    elif state == 0:
        (do something else)

I think this is more confusing, and I'm not sure why this would be changed. Would it be possible to restore the Qt5 behavior where Qt.Checked/Unchecked/PartiallyChecked are equal to whatever QCheckBox.stateChanged emits? 

OpenGL version: 3.3.0 NVIDIA 497.29
OpenGL renderer: NVIDIA GeForce RTX 2070/PCIe/SSE2
OpenGL vendor: NVIDIA Corporation

Locale: en_US.cp1252
Qt version: PyQt6 6.2.3, Qt 6.2.3
Qt platform: windows

Manufacturer: To Be Filled By O.E.M.
Model: To Be Filled By O.E.M.
OS: Microsoft Windows 10 Home (Build 19044)
Memory: 17,107,529,728
MaxProcessMemory: 137,438,953,344
CPU: 16 AMD Ryzen 7 2700X Eight-Core Processor         
OSLanguage: en-US

Installed Packages:
    alabaster: 0.7.12
    appdirs: 1.4.4
    Babel: 2.9.1
    backcall: 0.2.0
    blockdiag: 3.0.0
    certifi: 2021.10.8
    cftime: 1.6.0
    charset-normalizer: 2.0.12
    ChimeraX-AddCharge: 1.2.3
    ChimeraX-AddH: 2.1.11
    ChimeraX-AlignmentAlgorithms: 2.0
    ChimeraX-AlignmentHdrs: 3.2.1
    ChimeraX-AlignmentMatrices: 2.0
    ChimeraX-Alignments: 2.2.3
    ChimeraX-AlphaFold: 1.0
    ChimeraX-AltlocExplorer: 1.0.1
    ChimeraX-AmberInfo: 1.0
    ChimeraX-Arrays: 1.0
    ChimeraX-Atomic: 1.36.2
    ChimeraX-AtomicLibrary: 6.1.1
    ChimeraX-AtomSearch: 2.0.1
    ChimeraX-AxesPlanes: 2.1
    ChimeraX-BasicActions: 1.1
    ChimeraX-BILD: 1.0
    ChimeraX-BlastProtein: 2.0
    ChimeraX-BondRot: 2.0
    ChimeraX-BugReporter: 1.0
    ChimeraX-BuildStructure: 2.6.1
    ChimeraX-Bumps: 1.0
    ChimeraX-BundleBuilder: 1.1
    ChimeraX-ButtonPanel: 1.0
    ChimeraX-CageBuilder: 1.0
    ChimeraX-CellPack: 1.0
    ChimeraX-Centroids: 1.2
    ChimeraX-ChemGroup: 2.0
    ChimeraX-Clashes: 2.2.2
    ChimeraX-ColorActions: 1.0
    ChimeraX-ColorGlobe: 1.0
    ChimeraX-ColorKey: 1.5.1
    ChimeraX-CommandLine: 1.2.1
    ChimeraX-ConnectStructure: 2.0.1
    ChimeraX-Contacts: 1.0
    ChimeraX-Core: 1.4.dev202203120056
    ChimeraX-CoreFormats: 1.1
    ChimeraX-coulombic: 1.3.2
    ChimeraX-Crosslinks: 1.0
    ChimeraX-Crystal: 1.0
    ChimeraX-CrystalContacts: 1.0
    ChimeraX-DataFormats: 1.2.2
    ChimeraX-Dicom: 1.0
    ChimeraX-DistMonitor: 1.1.5
    ChimeraX-Dssp: 2.0
    ChimeraX-EMDB-SFF: 1.0
    ChimeraX-ExperimentalCommands: 1.0
    ChimeraX-FileHistory: 1.0
    ChimeraX-FunctionKey: 1.0
    ChimeraX-Geometry: 1.1
    ChimeraX-gltf: 1.0
    ChimeraX-Graphics: 1.1
    ChimeraX-Hbonds: 2.1.2
    ChimeraX-Help: 1.2
    ChimeraX-HKCage: 1.3
    ChimeraX-IHM: 1.1
    ChimeraX-ImageFormats: 1.2
    ChimeraX-IMOD: 1.0
    ChimeraX-IO: 1.0.1
    ChimeraX-ItemsInspection: 1.0
    ChimeraX-Label: 1.1
    ChimeraX-ListInfo: 1.1.1
    ChimeraX-Log: 1.1.5
    ChimeraX-LookingGlass: 1.1
    ChimeraX-Maestro: 1.8.1
    ChimeraX-Map: 1.1
    ChimeraX-MapData: 2.0
    ChimeraX-MapEraser: 1.0
    ChimeraX-MapFilter: 2.0
    ChimeraX-MapFit: 2.0
    ChimeraX-MapSeries: 2.1
    ChimeraX-Markers: 1.0
    ChimeraX-Mask: 1.0
    ChimeraX-MatchMaker: 2.0.6
    ChimeraX-MDcrds: 2.6
    ChimeraX-MedicalToolbar: 1.0.1
    ChimeraX-Meeting: 1.0
    ChimeraX-MLP: 1.1
    ChimeraX-mmCIF: 2.7
    ChimeraX-MMTF: 2.1
    ChimeraX-Modeller: 1.5.4
    ChimeraX-ModelPanel: 1.3.2
    ChimeraX-ModelSeries: 1.0
    ChimeraX-Mol2: 2.0
    ChimeraX-Morph: 1.0
    ChimeraX-MouseModes: 1.1
    ChimeraX-Movie: 1.0
    ChimeraX-Neuron: 1.0
    ChimeraX-Nucleotides: 2.0.2
    ChimeraX-OpenCommand: 1.8
    ChimeraX-PDB: 2.6.6
    ChimeraX-PDBBio: 1.0
    ChimeraX-PDBLibrary: 1.0.2
    ChimeraX-PDBMatrices: 1.0
    ChimeraX-PickBlobs: 1.0
    ChimeraX-Positions: 1.0
    ChimeraX-PresetMgr: 1.1
    ChimeraX-PubChem: 2.1
    ChimeraX-ReadPbonds: 1.0.1
    ChimeraX-Registration: 1.1
    ChimeraX-RemoteControl: 1.0
    ChimeraX-ResidueFit: 1.0
    ChimeraX-RestServer: 1.1
    ChimeraX-RNALayout: 1.0
    ChimeraX-RotamerLibMgr: 2.0.1
    ChimeraX-RotamerLibsDunbrack: 2.0
    ChimeraX-RotamerLibsDynameomics: 2.0
    ChimeraX-RotamerLibsRichardson: 2.0
    ChimeraX-SaveCommand: 1.5
    ChimeraX-SchemeMgr: 1.0
    ChimeraX-SDF: 2.0
    ChimeraX-Segger: 1.0
    ChimeraX-Segment: 1.0
    ChimeraX-SelInspector: 1.0
    ChimeraX-SeqView: 2.4.8
    ChimeraX-Shape: 1.0.1
    ChimeraX-Shell: 1.0
    ChimeraX-Shortcuts: 1.1
    ChimeraX-ShowAttr: 1.0
    ChimeraX-ShowSequences: 1.0
    ChimeraX-SideView: 1.0
    ChimeraX-Smiles: 2.1
    ChimeraX-SmoothLines: 1.0
    ChimeraX-SpaceNavigator: 1.0
    ChimeraX-StdCommands: 1.8
    ChimeraX-STL: 1.0
    ChimeraX-Storm: 1.0
    ChimeraX-StructMeasure: 1.0.1
    ChimeraX-Struts: 1.0.1
    ChimeraX-Surface: 1.0
    ChimeraX-SwapAA: 2.0
    ChimeraX-SwapRes: 2.1.1
    ChimeraX-TapeMeasure: 1.0
    ChimeraX-Test: 1.0
    ChimeraX-themes: 0.3.1
    ChimeraX-Toolbar: 1.1
    ChimeraX-ToolshedUtils: 1.2.1
    ChimeraX-Tug: 1.0
    ChimeraX-UI: 1.16.2
    ChimeraX-uniprot: 2.2
    ChimeraX-UnitCell: 1.0
    ChimeraX-ViewDockX: 1.1.2
    ChimeraX-VIPERdb: 1.0
    ChimeraX-Vive: 1.1
    ChimeraX-VolumeMenu: 1.0
    ChimeraX-VTK: 1.0
    ChimeraX-WavefrontOBJ: 1.0
    ChimeraX-WebCam: 1.0
    ChimeraX-WebServices: 1.0
    ChimeraX-Zone: 1.0
    colorama: 0.4.4
    comtypes: 1.1.10
    cxservices: 1.1
    cycler: 0.11.0
    Cython: 0.29.26
    debugpy: 1.5.1
    decorator: 5.1.1
    dill: 0.3.4
    docutils: 0.17.1
    entrypoints: 0.4
    filelock: 3.4.2
    fonttools: 4.30.0
    funcparserlib: 1.0.0a0
    grako: 3.16.5
    h5py: 3.6.0
    html2text: 2020.1.16
    idna: 3.3
    ihm: 0.26
    imagecodecs: 2021.11.20
    imagesize: 1.3.0
    ipykernel: 6.6.1
    ipython: 7.31.1
    ipython-genutils: 0.2.0
    jedi: 0.18.1
    Jinja2: 3.0.3
    joblib: 1.1.0
    jupyter-client: 7.1.0
    jupyter-core: 4.9.2
    kiwisolver: 1.3.2
    line-profiler: 3.4.0
    lxml: 4.7.1
    lz4: 3.1.10
    MarkupSafe: 2.1.0
    matplotlib: 3.5.1
    matplotlib-inline: 0.1.3
    msgpack: 1.0.3
    multiprocess: 0.70.12.2
    nest-asyncio: 1.5.4
    netCDF4: 1.5.8
    networkx: 2.6.3
    numexpr: 2.8.1
    numpy: 1.22.1
    openvr: 1.16.802
    packaging: 21.3
    ParmEd: 3.4.3
    parso: 0.8.3
    pathos: 0.2.8
    pdb-objects: 0.1.3
    pickleshare: 0.7.5
    Pillow: 9.0.0
    pip: 21.3.1
    pkginfo: 1.8.2
    pox: 0.3.0
    ppft: 1.6.6.4
    prompt-toolkit: 3.0.28
    psutil: 5.9.0
    pycollada: 0.7.2
    pydicom: 2.2.2
    Pygments: 2.11.2
    PyOpenGL: 3.1.5
    PyOpenGL-accelerate: 3.1.5
    pyparsing: 3.0.7
    PyQt6-commercial: 6.2.3
    PyQt6-sip: 13.2.0
    PyQt6-WebEngine-commercial: 6.2.1
    python-dateutil: 2.8.2
    pytz: 2021.3
    pywin32: 303
    pyzmq: 22.3.0
    qtconsole: 5.2.2
    QtPy: 2.0.1
    RandomWords: 0.3.0
    requests: 2.27.1
    scikit-learn: 1.0.2
    scipy: 1.7.3
    Send2Trash: 1.8.0
    SEQCROW: 1.4a2
    setuptools: 59.8.0
    sfftk-rw: 0.7.1
    six: 1.16.0
    snowballstemmer: 2.2.0
    sortedcontainers: 2.4.0
    Sphinx: 4.3.2
    sphinx-autodoc-typehints: 1.15.2
    sphinxcontrib-applehelp: 1.0.2
    sphinxcontrib-blockdiag: 3.0.0
    sphinxcontrib-devhelp: 1.0.2
    sphinxcontrib-htmlhelp: 2.0.0
    sphinxcontrib-jsmath: 1.0.1
    sphinxcontrib-qthelp: 1.0.3
    sphinxcontrib-serializinghtml: 1.1.5
    suds-community: 1.0.0
    tables: 3.7.0
    threadpoolctl: 3.1.0
    tifffile: 2021.11.2
    tinyarray: 1.2.4
    tornado: 6.1
    traitlets: 5.1.1
    urllib3: 1.26.8
    wcwidth: 0.2.5
    webcolors: 1.11.1
    wheel: 0.37.1
    wheel-filename: 1.3.0
    WMI: 1.5.1

[Eric Pettersen]

Hi Tony,

I think the signal value has always been an integer. The problem is that CheckState changed to a Python enum in PyQt6. I think you can get the code to work as:

if state == Qt.Checked.value:

...

But that isn't a great solution since I believe that only PyQt6 is treating CheckState as a Python enum, so that same code will not work in PyQt5 nor in PySide6, both of which treat CheckState as an int AFAIK, which defeats the purpose of our Qt shim. I think this is just a bad decision on PyQt6's part, but we're going to need to figure out if there's something we can do in the shim to work around this issue.

[Tom Goddard]

This is a change in PyQt6. Apparently the idea is PyQt6 code would need to look like

def some_function(state):
    if state == Qt.CheckState.Checked.value:
        (do something)
    elif state == Qt.CheckState.Unchecked.value:
        (do something else)

as shown in an example on this page ​https://zetcode.com/pyqt6/widgets/
but that code will not work in PyQt5 where Qt.CheckState.Checked is an integer (and has no "value" attribute). Notice I also put in the enumeration name "CheckState" in Qt.CheckState.Checked, because in PyQt6 that is also required. The shorter version Qt.Checked only exists because the ChimeraX Qt shim added it. But it is another breaking change in PyQt6 vs PyQt5. This is a major version change from 5 to 6 so such breaking changes are allowed, and apparently Phil Thompson who makes PyQt thought they were a good idea.

It is also somewhat bizarre that the QCheckBox.stateChange signal is defined in Qt to return an "int" not a Qt.CheckState. That seems like a Qt bug, but it is how it is documented. I guess it didn't matter since Qt C++ enumeration values are comparable to int. Maybe the Qt signal machinery can't handle enum argument types -- not sure.

At any rate, I don't think it is wise for us to start tampering with enumeration values in the ChimeraX Qt shim, for instance adding a method eq to compare it to integers. Looking at how ChimeraX code avoids this issue I see we ask the QCheckBox instance its state rather than using the int callback value.

def some_function(self, *args):
    checked = self._checkbutton.checkState()
    if checked == Qt.CheckState.Checked:
        (do something)
    elif checked == Qt.CheckState.Unchecked:
        (do something else)

[Tom Goddard]

If you are not using tristate (allowing partially checked) buttons, then this code is also usable and slightly simpler

def some_function(self, *args):
    if self._checkbutton.isChecked():
        (do something)
    else:
        (do something else)

[Tony Schaefer]

Thanks for providing clarity and suggestions. I don't think I use tristate checkboxes anywhere, I think I'll go with

```
def some_function(state):
    if state:
        (checked code)
    else:
        (unchecked code)
```

This makes enough sense to me, and it should work with Qt 5 and 6 as long as state remains an int. Using checkbox.isChecked() or checkbox.checkState() is definitely easier to read, but it feels weird to check the state again when that info should already be passed to the function. Knowing me, I'd probably go back to that code in a few months, wonder why I'm not just using 'state', and try to "fix" it.

Thanks,

Tony

[goddard@…]

I think the code that is most readable and least likely to break in the future would use

  if self._mycheckbox.checkState() == Qt.Unchecked:

Qt is C++ and generally you would not depend on the enumeration integer values to be stable -- the idea is you use the enumeration symbols instead.  But I realize shaky code often will hard code the integer values and the Qt documentation does state the integer values, so they probably will never be changed.  Still it makes sense to use the symbols instead of the integers for clarity of code.

[Eric Pettersen]

Hi Tony,

We just added functions to the Qt shim to make it possible to handle these situations. The functions are qt_enum_as_int() and qt_enum_from_int(). The former just takes the int or enum value as an argument, but latter takes the enum class and the value as arguments, since it may have to return the enum instance. If you choose to use the shim functions then your code would look like:

from Qt.QtCore import Qt
from Qt import qt_enum_from_int

def some_function(state):

if qt_enum_from_int(Qt.CheckState, state) == Qt.Checked:

...

It is kind of verbose.

--Eric

[Eric Pettersen]

Those shim functions are in tomorrow's build.

[Tony Schaefer]

Taking Tom's comment into consideration and if the shim functions are only going to be in the ChimeraX 1.4 release/daily builds, it might be best to make a separate GitHub branch for SEQCROW that requires ChimeraX 1.4. If I'm making changes, I might as well stick to the PyQt6 standard. I've got a local version of SEQCROW that works with the 3/12 daily build. I still need to work on some upcoming SEQCROW tools using ChimeraX < 1.4 because that's what we have installed on our computing cluster. It can take a while to get new software installed. Depending on what happens first (ChimeraX 1.4 release or new SEQCROW tools finished), I can just put the corresponding branch on the toolshed and reconcile the differences later.

I haven't encountered enum before, but I'm not sure why PyQt6 didn't use IntFlag or IntEnum for this stuff. They seem to fit the bill better than a regular Enum. You don't need to grab the "value" to see if they are equal to an int.

Thanks,

Tony

[goddard@…]

There is an ugly complication here.  Qt 6 does is not supported on CentOS 7 and Ubuntu 16 and 18, so we are thinking of providing ChimeraX 1.4 with Qt 5 in those old Linux distributions and Qt 6 on Mac, Windows and Ubuntu 20 and CentOS 8.  For that to work the code needs to work in both Qt 5 and Qt 6.  That has not been too hard in the main ChimeraX code.  But it will cause problems if SEQCROW bundles cannot say it depends on Qt 5 or Qt 6.  I'd encourage you to use code that will work with Qt 5 and Qt 6 and what I suggested for QCheckBox callbacks works for both and is what ChimeraX itself uses.  But if you don't want to we will need to discuss our ChimeraX distribution plan.  Maybe it would make sense to change it to make ChimeraX 1.4 be Qt 5 on all platforms, but immediately also put out ChimeraX 1.5 with Qt 6 on the supported platforms.  Another aspect of our plan is to drop support for older Linux systems where Qt 6 is not supported after ChimeraX 1.4.

[Tony Schaefer]

That sounds complicated. I'll just implement a method that works for both Qt versions. I don't want to worry about having two versions of SEQCROW for concurrent ChimeraX releases. Our cluster uses CentOS 7. I'm not sure when they're updating it, but I'll need SEQCROW to work with it.

Apparently, I can do Qt.CheckState(state) == Qt.Checked in both versions. It works if 'state' is an int, like from the stateChanged signal, or a member of the Qt.CheckState enumeration. With this, I'm not calling a function to get data that I already have, and it's about as readable as the original.

Best,

Tony

[Tom Goddard]

I don't understand your strong desire to use the int argument of this badly defined Qt API. There is zero overhead to looking up the checkbutton state, and your proposal is calling a CheckState constructor function. But of course do it as you see fit.

I am more concerned to hear you need ChimeraX on CentOS 7 since our plan was to drop CentOS 7 support after ChimeraX 1.4 because Qt 6 does not support it. CentOS 7 end of life (no security patches) is in about 2 years (June 2024). If we don't have new ChimeraX releases beyond 1.4 for CentOS 7 will this be a major problem for you? We love your work on SEQCROW and will rethink our plan (maybe delay Qt 6 to ChimeraX 1.5 near end of 2022?) if this is a big problem.

[Tony Schaefer]

Tom,

I appreciate your concern, but using ChimeraX 1.3 or older should be fine for my needs. I have a local version of SEQCROW that works simultaneously with ChimeraX 1.2.5 (version installed on our cluster), and the 1.4 daily build from 3/12. There are some features I like in 1.4 (e.g. independent rotation for tiled structures, changing model names on the model panel). However, using 1.2.5 or 1.3 isn't a major issue for me.

I might be annoyed by this stateChanged inconsistency, but it's really only ~50 lines of code. I also had to make a few other changes. QComboBox.findData seems to have changed "role" or "flags"  from a keyword argument to a regular argument (doesn't seem to be updated in the documentation). That was another 10 lines of code. There's also a tool where I accidentally used non-consecutive indices when inserting widgets on a QBoxLayout. Qt5 didn't care, but in Qt6 it caused ChimeraX to crash. There was also a non-Qt issue with matplotlib changing how colormaps work. Getting SEQCROW to run with both ChimeraX 1.2.5/1.3 and the 1.4 daily build was not too difficult. SEQCROW 1.4 will work with both Qt5 and Qt6.

Dr. Wheeler is thinking about using ChimeraX/SEQCROW instead of GaussView for a computational quantum chemistry course next Fall. He'll just have to keep in mind that ChimeraX 1.4/1.5 might not run on the teaching cluster (different than the main cluster, usually everything is older). The class sticks to basic computations: energies, geometry optimizations, vibrational modes, transition states, and open shell systems. These are all things you can do with SEQCROW 1.3.2 in ChimeraX 1.3. The features being added in SEQCROW 1.4 will help locate transition state structures and allow users to work more efficiently on a cluster without resorting to bash/tcsh/etc.

SEQCROW 1.4 will have 1-3 new tools - all would reuse code from tools already in SEQCROW. The major changes will not be in GUI's. If in the future I need to make some wacky tool that is painful to get working in Qt5 and Qt6, I may re-evaluate Qt5 support for SEQCROW > 1.4. SEQCROW 1.5 probably won't come for some time, but I can't think of any tools I want to add that would require Qt features I haven't used before.

Best,

Tony
________________________________
From: ChimeraX <ChimeraX-bugs-admin@cgl.ucsf.edu>
Sent: Wednesday, March 16, 2022 12:56 PM
Cc: chimera-programmers@cgl.ucsf.edu <chimera-programmers@cgl.ucsf.edu>; Anthony James Schaefer <tony.schaefer@uga.edu>; zjp@cgl.ucsf.edu <zjp@cgl.ucsf.edu>
Subject: Re: [ChimeraX] #6370: Breaking QCheckbox stateChanged signal change in Qt6

[EXTERNAL SENDER - PROCEED CAUTIOUSLY]


#6370: Breaking QCheckbox stateChanged signal change in Qt6
-------------------------------------+--------------------------
          Reporter:  Tony Schaefer   |      Owner:  Zach Pearson
              Type:  defect          |     Status:  assigned
          Priority:  normal          |  Milestone:
         Component:  Window Toolkit  |    Version:
        Resolution:                  |   Keywords:
        Blocked By:                  |   Blocking:
Notify when closed:                  |   Platform:  all
           Project:  ChimeraX        |
-------------------------------------+--------------------------

Comment (by Tom Goddard):

 I don't understand your strong desire to use the int argument of this
 badly defined Qt API.  There is zero overhead to looking up the
 checkbutton state, and your proposal is calling a CheckState constructor
 function.  But of course do it as you see fit.

 I am more concerned to hear you need ChimeraX on CentOS 7 since our plan
 was to drop CentOS 7 support after ChimeraX 1.4 because Qt 6 does not
 support it.  CentOS 7 end of life (no security patches) is in about 2
 years (June 2024).  If we don't have new ChimeraX releases beyond 1.4 for
 CentOS 7 will this be a major problem for you?  We love your work on
 SEQCROW and will rethink our plan (maybe delay Qt 6 to ChimeraX 1.5 near
 end of 2022?) if this is a big problem.

--
Ticket URL: <https://www.rbvi.ucsf.edu/trac/ChimeraX/ticket/6370#comment:13>
ChimeraX <https://www.rbvi.ucsf.edu/chimerax/>
ChimeraX Issue Tracker

[goddard@…]

Thanks for the info on the status of SEQCROW and Qt 5 vs 6 issues.  Our plan is still to release ChimeraX 1.4 probably in late May or June with Qt 5 for CentOS 7 and with Qt 6 for other platforms but I will let you know if we change that.

[Zach Pearson]

Closing for now since a workaround has been decided on.

If at any point you feel like you'd like to change course, please feel free to reopen this ticket or email me directly, and we can work together on other solutions. :)