PR: Support for sets in the Variable Explorer

[Prikers]

Fixes #2355

---

set is now a supported type in the variable explorer. Now I chose the Qt.darkGreen color as it was not already used for other objects: if that is not ok there are some other colors left 😄

[pep8speaks]

Hello @Prikers! Thanks for updating the PR.

• In the file spyder/widgets/variableexplorer/collectionseditor.py, following are the PEP8 issues :

Line 133:11: E127 continuation line over-indented for visual indent

Comment last updated on September 20, 2017 at 20:02 Hours UTC

[dalthviz]

Thanks @Prikers!, I tested locally and the cases you provided work as expected 👍

[rlaverde]

Works as expected :-)

[rlaverde]

I think that sets should be editable, How difficult will be to edit sets? (I tried changing readonly to True, but I convert sets to lists.)

[goanpeca]

I think that sets should be editable

@rlaverde sets cannot be edited by index:

python
a = {1, 2}
a[0]

Traceback (most recent call last):

  File "<ipython-input-10-5ccf417d7af1>", line 1, in <module>
    a[0]

TypeError: 'set' object does not support indexing

a[0] = 2
Traceback (most recent call last):

  File "<ipython-input-11-48f8c1dbf98d>", line 1, in <module>
    a[0] = 2

TypeError: 'set' object does not support item assignmen

So I don't think we really need to support anything beyond remove or adding items to it.

[goanpeca]

@Prikers thanks for the work. Now, why does simple_set says List on the header, also, sets by definition are not ordered containers, so displaying an index is not correct cause sets are not even indexable. However I think users would expect to be able o add and remove items, any chance we could include that?

[rlaverde]

sets cannot be edited by index:

But you could replace an element

In[8]: a = {10, 12, 15}
In[9]: a.remove(15)
In[10]: a.add(23)
In[11]: a
Out[12]: {10, 12, 23}

I don't know how the internals of variable explorer works, and how easy something like this could be do it

[goanpeca]

I don't know how the internals of variable explorer works, and how easy something like this could be do it

That is what I am suggesting, removing and adding is not the same as editing. And emulating that behavior (editing in place) is conceptually wrong and leads to believing they can be accessed by index.

[Prikers]

1. Hum... Good catch @goanpeca regarding the title! It is actually more subtle than I anticipated. The problem is that, as you said, set does not support indexing. The code heavily depends on indexes for displaying collections so I had converted set into a list under the hood in order not to reimplement everything. Turns out that is why the title is List. I don't know if turning the set into a list was the best option. What do you think? Apart from the title (which could be manually fixed) is it a bad idea?

2. @rlaverde: It was on purpose that I made it not editable, but you are right, unlike tuple we can add or remove elements. This should definitely be implemented. Thanks for catching that!

[goanpeca]

I now do not think turning the set into a list is the best option. What do you think? Apart from the title (which could be manually fixed) is it a bad idea?

I understand the temptation to transform into a list, but I think we should not do this but generalize the concept for a set or even create a new editor just for handling sets, which is a non indexable non sorted, mutable container

[Prikers]

That's wise 😄
Then should I create a new file seteditor.py to create a new editor or simply adjust the collectioneditor to handle the non indexable objects by checking if isinstance(data, set) whenever it is necessary?

[goanpeca]

I think we should be able to use the collections editor. But instead of checking every time if it is a given object, I think it would be better to define characteristics for an object and act accordingly.

is_mutable
is_ordered

But what do you think @ccordoba12?

[ccordoba12]

I think what @Prikers has done is the best we can do for now. Before merging, I'd like to see these improvements :

1. Use Set instead of List in the title of the viewer (as already mentioned).
2. Remove the Index column when displaying sets.

[ccordoba12]

That is what I am suggesting, removing and adding is not the same as editing.

None of our editors allow to add/remove new content to an object, so what @Prikers did (making the contents read-only) is the right thing to do for sets.

But instead of checking every time if it is a given object, I think it would be better to define characteristics for an object and act accordingly.

My plan for beta2 or beta3 is to use the multipledispatch library to define the behavior of several functions in the Variable Explorer according to the object's type.

[ccordoba12]

multipledispatch will also allow to easily extend the Variable Explorer because third-party authors will only need to register viewers and display values according to the object type they want to be shown.

[Prikers]

Use Set instead of List in the title of the viewer (as already mentioned).
Remove the Index column when displaying sets.

Will do!

None of our editors allow to add/remove new content to an object

That's what I thought but it turns out it is possible 😮
Anyway if the whole thing will be refactored it may not be necessary to implement too many features

[Prikers]

Exploring a bit more I noted a bug:

• Create a tuple objet
• Open it in the variable explorer
• Right click on a line to get the same menu as in the above gif
• Click on rename or duplicate gerenates an error in the internal console

  File "C:\...\spyder\widgets\variableexplorer\collectionseditor.py", line 978, in rename_item
    self.copy_item(True)
  File "C:\...\spyder\widgets\variableexplorer\collectionseditor.py", line 961, in copy_item
    QLineEdit.Normal, orig_key)
TypeError: getText(QWidget, str, str, echo: QLineEdit.EchoMode = QLineEdit.Normal, text: str = '', flags: Union[Qt.WindowFlags, Qt.WindowType] = Qt.WindowFlags(), inputMethodHints: Union[Qt.InputMethodHints, Qt.InputMethodHint] = Qt.ImhNone): argument 5 has unexpected type 'int'

[ccordoba12]

@Prikers, for now please finish your support for sets with my recommendations. Then you can open a new PR to fix the error you found and add support to add/remove elements to sets.

[Prikers]

[sorry for the delay] I have hidden the first column (Index) for sets and corrected the issue with the title. I will create another issue/PR for the add/remove support.

Now I noticed that Set is not part of the .pot files for translations. I have locally another commit with the spyder/locale/spyder.pot file modified (and the one in French as well) but I am not sure I should do it. Are these .pot files automatically generated at some specific point in time?

[ccordoba12]

Now I noticed that Set is not part of the .pot files for translations.

Translations will be updated when we are about to release Spyder 4, to not ask our translators to constantly update a moving target (because we can add/remove a lot of translation strings during our betas).

So, don't worry about this ;-)

[ccordoba12]

Great work @Prikers! Thanks a lot for your contribution!!