UrlList: allow removal of url from the list

[payno]

when the UrlList is set editable the selection mode is set to qt.QAbstractItemView.ExtendedSelection

Here are two screenshots of the feature:

• removing a single item
  
• removing several items ()
  

closes #3911

Changelog:

• gui.ImageStack.UrlList: allow removal of url from the list (if the list is editable only)

[vallsv]

Could you put an upper case to Remove?

I also think the change of the selection mode is a bit weird. But maybe you can wait for Thomas feedback.

Also optionally you could as a del keyboard short cut (if it's not already the case), so what you already can have something fast to use.

[payno]

Thanks for the review. I rename remove and added a shortcut to the action.

I agree that changing the selection mode is weird. But being able to select several item in 'non-editable' mode is weird as well.
Maybe the better solution could be to define the mode in the constructor and avoid changing it... and get a single mode for the entire widget life. To be see with @t20100 no hurry for having it right away (but if we can have it for the 2.0 it would be great...)

[vallsv]

I have updated the description to put the Changelog as a single item. I think it is used and extracted by @t20100 scripts, so better to normalize that.

[t20100]

I left a few comments.
I would not modify the selection mode.

[t20100]

I agree that changing the selection mode is weird. But being able to select several item in 'non-editable' mode is weird as well.
Maybe the better solution could be to define the mode in the constructor and avoid changing it... and get a single mode for the entire widget life.

Is UrlList part of the public API?
If so, I'd keep it generic and so not change the selection mode when switching editable mode. It would also be best located under silx.gui.widgets.
If it is private, it would be best to rename it _UrlList (with a deprecated UrlList) and then it's behavior can be tuned to whatever ImageStack needs.

[payno]

I agree that changing the selection mode is weird. But being able to select several item in 'non-editable' mode is weird as well.
Maybe the better solution could be to define the mode in the constructor and avoid changing it... and get a single mode for the entire widget life.

Is UrlList part of the public API? If so, I'd keep it generic and so not change the selection mode when switching editable mode. It would also be best located under silx.gui.widgets. If it is private, it would be best to rename it _UrlList (with a deprecated UrlList) and then it's behavior can be tuned to whatever ImageStack needs.

For me it should be public because it can have other usage.
When you say 'so not change the selection mode when switching editable mode' you mean that we should remove the 'setEditable' function from the public API- and force the widget to keep the same 'selection mode' during the entire life right ?

[t20100]

you mean that we should remove the 'setEditable' function from the public API- and force the widget to keep the same 'selection mode' during the entire life right ?

No, I mean remove calls to setSelectionMode from setEditable and leave it to the user to choose how to handle the selection mode: You might want to keep the multiple selection enabled when not editable if urls corresponds to curves you want to plot or datasets you want to process for example, so I would not make a link between editable and selection mode.

And if ImageStack needs to do so, it can inherit and override setEditable.

[payno]

I move UrlList to silx.gui.widgets As this is the 2.0 and as I think I am the only one using it I didn't add a deprecation warning. Please let me know if you think we should have one.

[vallsv]

Be careful UrlList is used by silx compare no?
BUt yes, it's a good location.

[payno]

Be careful UrlList is used by silx compare no? BUt yes, it's a good location.

no silx compare is using UrlSelectionTable. From what I see only ImageStack is using the UrlList

[t20100]

I left a few last comments.
I would be good to add UrlList to the API documentation and evenutally add missing docstrings for public members

[t20100]

@payno there's a conflict to merge this + an issue spotted by CI:

venv_test\lib\site-packages\silx\gui\plot\test\testImageStack.py:38: in <module>
    from silx.gui.plot.ImageStack import ImageStack
venv_test\lib\site-packages\silx\gui\plot\ImageStack.py:150: in <module>
    class _ToggleableUrlSelectionTable(qt.QWidget):
venv_test\lib\site-packages\silx\gui\plot\ImageStack.py:201: in _ToggleableUrlSelectionTable
    def setUrls(self, urls: Iterable[DataUrl]):
E   TypeError: 'ABCMeta' object is not subscriptable

It sounds from __future__ import annotations is missing in silx\gui\plot\ImageStack.py

[payno]

missing import has been added and a rebase from the main has been done for the PR.