labscript-suite / runviewer

π—Ώπ˜‚π—»π˜ƒπ—Άπ—²π˜„π—²π—Ώ is a graphical interface for visualising hardware-timed experiments composed using the 𝘭𝘒𝘣𝘴𝘀𝘳π˜ͺ𝘱𝘡 𝘴𝘢π˜ͺ𝘡𝘦.
http://labscriptsuite.org
BSD 2-Clause "Simplified" License
2 stars 39 forks source link

Error when ticking/unticking multiple shots #7

Closed philipstarkey closed 7 years ago

philipstarkey commented 7 years ago

Original report (archived issue) by Shaun Johnstone (Bitbucket: shjohnst, GitHub: shjohnst).


Sometimes when ticking or unticking multiple shots, several error message popups appear, with the following exception:

#!python

Traceback (most recent call last):
  File "C:\labscript_suite\runviewer\__main__.py", line 351, in on_shot_selection_changed
    self.plot_items[channel][shot].setPen(pg.mkPen(QColor(colour), width=2))
TypeError: arguments did not match any overloaded call:
  QColor(Qt.GlobalColor): argument 1 has unexpected type 'function'
  QColor(int): too many arguments
  QColor(QVariant): argument 1 has unexpected type 'function'
  QColor(): argument 1 has unexpected type 'function'
  QColor(int, int, int, alpha: int = 255): argument 1 has unexpected type 'function'

The shots still get ticked/unticked despite the errors.

philipstarkey commented 7 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Hmm, I feel like I've seen and fixed this bug before...Yes, it was changed here (9c16be5f94791f0812ca4217d9997131c042982d) by @rpanderson. I feel like this is something to do with different versions of Qt.

We might just need to implement a check to see if it is a function or not so we can decide whether to call it or not.

Can you confirm what version of Qt and PyQt you are on?

philipstarkey commented 7 years ago

Original comment by Shaun Johnstone (Bitbucket: shjohnst, GitHub: shjohnst).


I'm on Qt version 4.8.7, PyQt version 4.11.4

philipstarkey commented 7 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Ok, I have a feeling that the change by Russ listed in my above comment was just wrong. As far as I can see, colour should be a function on that line, and so needs to be called as QColor(colour()). The only other place we pull that out of the model is line 359, which we then call on line 364. The other places we use the colour are just pulling it out of a dictionary that is created by the code that needs fixing, and so at that point they won't be functions, and will thus work correctly as is (provided we fix the offending line of code).

Feel free to change it back to the way it was and make a pull request (or include it with the pull request you have open).

If people raise the opposite problem as an issue, we'll set the minimum pyqt version and ask them to upgrade.

philipstarkey commented 7 years ago

Original comment by Shaun Johnstone (Bitbucket: shjohnst, GitHub: shjohnst).


Hmm now I sometimes get

#!python

Traceback (most recent call last):
  File "C:\labscript_suite\runviewer\__main__.py", line 350, in on_shot_selection_changed
    self.plot_items[channel][shot].setPen(pg.mkPen(QColor(colour()), width=2))
TypeError: arguments did not match any overloaded call:
  QColor(Qt.GlobalColor): argument 1 has unexpected type 'NoneType'
  QColor(int): too many arguments
  QColor(QVariant): argument 1 has unexpected type 'NoneType'
  QColor(): argument 1 has unexpected type 'NoneType'
  QColor(int, int, int, alpha: int = 255): argument 1 has unexpected type 'NoneType'

It seems to happen when checking/unchecking multiple shots which have been checked before?

philipstarkey commented 7 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Does it only happen if you do it quickly? It might be a race conditions somewhere that I haven't caught properly. Unfortunately I can't reproduce on my own PC. Could you try and come up with some reproducible steps?

philipstarkey commented 7 years ago

Original comment by Shaun Johnstone (Bitbucket: shjohnst, GitHub: shjohnst).


All I have to do is load a single shot in, then tick it. If I then untick it, I get an error pop up, then every subsequent time I tick it I get 2 error popups and one for every untick.

This is with the above change of colour --> colour() on line 350 of my branch

philipstarkey commented 7 years ago

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).


Ok, It must be to do with the slightly different PyQt versions we have. I'm on 4.11.1, which really shouldn't be having so much trouble with doing QColor(None). Apparently that doesn't work anymore!

I'm going to suggest we don't 'uncolour' a shot when we untick it. That way it will keep it's the same colour when you retick it.

Proposed patch for the on_shot_selection_changed method:

#!python

        if self.shot_model.indexFromItem(item).column() == SHOT_MODEL__CHECKBOX_INDEX:

            # add or remove a colour for this shot
            checked = item.checkState()
            row = self.shot_model.indexFromItem(item).row()
            colour_item = self.shot_model.item(row, SHOT_MODEL__COLOUR_INDEX)

            if checked:            
                colour = colour_item.data(Qt.UserRole)
                if qt_type == 'PyQt4':
                    colour = colour.toPyObject()

                if colour is not None:
                    colour = colour()
                else:                    
                    colour = self.shot_colour_delegate.get_next_colour()

                colour_item.setEditable(True)
                pixmap = QPixmap(20, 20)
                pixmap.fill(colour)
                icon = QIcon(pixmap)
                colour_item.setData(lambda clist=self.shot_colour_delegate._colours, colour=colour: int_to_enum(clist, colour), Qt.UserRole)
                colour_item.setData(icon, Qt.DecorationRole)
            else:
                # colour = None
                # icon = None
                colour_item.setEditable(False)

            # model.setData(index, editor.itemIcon(editor.currentIndex()),
            # model.setData(index, editor.itemData(editor.currentIndex()), Qt.UserRole)

            self.update_channels_treeview()
        elif.....
philipstarkey commented 7 years ago

Original comment by Shaun Johnstone (Bitbucket: shjohnst, GitHub: shjohnst).


OK, that sounds good, updated in my pull request

philipstarkey commented 7 years ago

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).


Changed colour to colour() to fix issue #7

β†’ \<\<cset 4ed5a8d9fba4c8180a7b50084ea70dda1cd24873>>