issues with moving existing item in DictSyncModel

[SimonRenblad]

Bug Report

One-Line Summary

Identified 3 problems with moving an existing item in DictSyncModel.

Issue Details

While testing the InteractiveArgumentsDock, I identified 3 issues with moving an existing item in DictSyncModel. Note that currently no subclass in the codebase of DictSyncModel moves existing items in the model.

 def __setitem__(self, k, v):
        if k in self.backing_store:
            old_row = self.row_to_key.index(k)
            new_row = self._find_row(k, v)
            if old_row == new_row:
                self.dataChanged.emit(self.index(old_row, 0),
                                      self.index(old_row, len(self.headers)-1))
            else:
                self.beginMoveRows(QtCore.QModelIndex(), old_row, old_row,
                                   QtCore.QModelIndex(), new_row)
            self.backing_store[k] = v
            self.row_to_key[old_row], self.row_to_key[new_row] = \
                self.row_to_key[new_row], self.row_to_key[old_row]
            if old_row != new_row:
                self.endMoveRows()

1. Since _find_row returns len(self.row_to_key) if the item should be inserted at the last row, IndexError is raised when swapping old and new rows.
2. Swapping the old and new row directly may lead to an incorrectly sorted list.
3. When new_row = old_row + 1, beginMoveRows will fail, as the item should not move. From docs:

Note that if sourceParent and destinationParent are the same, you must ensure that the destinationChild is not within the range of sourceFirst and sourceLast + 1.

Steps to Reproduce

from artiq.gui.models import DictSyncModel

class Model(DictSyncModel):
    def __init__(self, init):
        DictSyncModel.__init__(self, ["priority"], init)

    def convert(self, k, v, column):
        if column == 0:
            return v
        else:
            raise ValueError

    def sort_key(self, k, v):
        return v

model = Model({'a': 1, 'c': 4, 'b': 3})

# the 3 issues, comment out the other two for reproducing a specific case
model['a'] = 10 # greater than 'c' -> throws IndexError

model['a'] = 2 # segmentation fault

model['c'] = 0 # expect: [c, a, b], observe: [c, b, a]  

Your System

• Operating System: NixOS
• ARTIQ version: ARTIQ-8