PR: Add edition menu and toolbar to dataframe viewer

[dpturibio]

Description of Changes

Creation of a toolbar and menu for edition of Dataframes:
New features: edit cell/header/index, duplicate row/column, insert a row/column after/before/above/bellow the selected one, remove row/column, and resize as you can see in screenshot.

Attempts of creating duplicated header/index names are treated and a suffix "_copy(n)", with n=0,1,2,... is concatenated in header/index name.

Issue(s) Resolved

Fixes spyder-ide/ux-improvements#76

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:
@dpturibio / Diego Prosperi Turibio

[ccordoba12]

Hey @dpturibio, thanks a lot for your contribution! Quick review according to the screenshot you posted above:

• Please move the Copy action to be below Edit. That will follow the same convention used in the lists viewer.

• Please add a submenu called Convert to, move there all actions that start with To (i.e. To bool, To complex, etc) but remove the To prefix from their names, and place it in a new section between the section that ends with Duplicate column and the one that starts with Resize rows to contents. In other words, the context menu should look like this:

  ...
  Duplicate column
  ----------------
  Convert to > Bool
               Complex
               ...
  ----------------
  Resize rows to contents
  ...
  

[dpturibio]

Thank you for your inputs @ccordoba12 . Bellow a screenshot of the new menu layout.

Please, let me know if you think any further change is necessary.

[ccordoba12]

That looks really nice! Thanks @dpturibio for your hard work on this!

I don't have additional comments about the context menu's new UI. I'll try to review your code in the following days.

[dpturibio]

Hello @ccordoba12. Just to let you know, I was doing some tests with this new implementation and found some untreated errors when duplicating rows with numerical indexes(before I had just tested strings).
So I corrected it in this new commit ok?

But for some reason, some tests for test_ipythonconsole.py started failing with timeout(as you can see in above commits). I tried to increase time to prevent it but unsuccessful. I do not think this is related to the changes I did, but anyway, I will wait for your inputs when you can review the code.

Thank you,

[dpturibio]

Hi @ccordoba12, how are you doing? Hope everything is ok. Just pinging you to put in queue this PR ok.
Thank you.

[dpizetta]

@ccordoba12, @dpturibio is part of our team here, could you take a look on this PR? Also, is it possible to add it to the next release of v5? Our project uses this editor and we are currently patching Spyder code to add it. Thanks :)

[ccordoba12]

@dpturibio and @dpizetta, after making the changes I mentioned, I was reviewing this manually and found that it's failing to add/remove rows and columns in some simple cases For instance:

• Removing the only column of df = pd.DataFrame([1,2,3]) gives this error:

  Traceback (most recent call last):
    File "/home/carlos/Projects/spyder/spyder/spyder/plugins/variableexplorer/widgets/dataframeeditor.py", line 1256, in headerData
      header = self.model.header(self.axis, section)
    File "/home/carlos/Projects/spyder/spyder/spyder/plugins/variableexplorer/widgets/dataframeeditor.py", line 222, in header
      return ax[x]
  IndexError: list index out of range

• Trying to add a row to that dataframe with Insert above or Insert below gives this error:

  Traceback (most recent call last):
    File "/home/carlos/Projects/spyder/spyder/spyder/plugins/variableexplorer/widgets/dataframeeditor.py", line 689, in <lambda>
      triggered=lambda: self.insert_item(axis=1, before_above=True)
    File "/home/carlos/Projects/spyder/spyder/spyder/plugins/variableexplorer/widgets/dataframeeditor.py", line 932, in insert_item
      new_row.axes[0].values[0] = new_name
  ValueError: invalid literal for int() with base 10: 'new_row'

So I think this needs more work and also effort because I'd like to see tests for the new functionality added here. Given that, I think it's better to leave it for Spyder 6 (which will be released before the end of the year) to not introduce changes that will force us to put more time into maintaining Spyder 5.

What do you think?

[dpizetta]

@ccordoba12 thanks for your feedback on this. It seems fair. @dpturibio is fixing the problems you mentioned, but tests are nice to keep everything under control.

[pep8speaks]

Hello @dpturibio! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file spyder/utils/icon_manager.py:

Line 205:80: E501 line too long (99 > 79 characters)
Line 206:80: E501 line too long (98 > 79 characters)
Line 210:80: E501 line too long (85 > 79 characters)
Line 211:80: E501 line too long (88 > 79 characters)
Line 214:80: E501 line too long (98 > 79 characters)
Line 215:80: E501 line too long (101 > 79 characters)

Comment last updated at 2023-11-07 20:53:42 UTC

[dpizetta]

Some usability comments:

• When adding row/column, the selection should go to the new item (check Sheets/Excel);
• When removing row/column, the selection should go back to the previous item (check Sheets/Excel)
• Edit button does not work for headers, but I think it should;
• Should selecting headers enable the same drodown menu (add/remove/etc)? I think so (check Sheets/Excel)
• Enable/disable toolbar options when there is no selection, or when the selection allow/not allow the tool;

[dpturibio]

Some usability comments:

• When adding row/column, the selection should go to the new item (check Sheets/Excel);
• When removing row/column, the selection should go back to the previous item (check Sheets/Excel)
• Edit button does not work for headers, but I think it should;
  Comment: you can edit headers using right mouse button
• Should selecting headers enable the same drodown menu (add/remove/etc)? I think so (check Sheets/Excel)
  Comment: probably not, as you can only select the headers when you have multiIndex, otherwise they behave like a sort button
• Enable/disable toolbar options when there is no selection, or when the selection allow/not allow the tool;
  working on it

[dpturibio]

Some usability comments:

* When adding row/column, the selection should go to the new item (check Sheets/Excel);

* When removing row/column, the selection should go back to the previous item (check Sheets/Excel)

* Edit button does not work for headers, but I think it should;

* Should selecting headers enable the same drodown menu (add/remove/etc)? I think so (check Sheets/Excel)

* Enable/disable toolbar options when there is no selection, or when the selection allow/not allow the tool;

Hello @ccordoba12 and @dpizetta, I finished the changes in dataframe editor as proposed by Daniel in the linked message. I created also the automated tests in test_dataframeeditor.py.
Please let me know if you need any further changes/modifications in the code or tests.
Thank you so far for the assistance,
Diego.

[ccordoba12]

@dpturibio, thanks for all your hard work here! I just pushed two commits to your branch: one to simplify the new test you added (thanks a lot for it!) and the other with code style improvements for your new code.

And I have two final comments that I'd like to see addressed before merging:

1. I think there are no tests for the functionality you added in commit d8264db, which allows users to duplicate several rows/columns at the same time (if I understand things correctly). If that's the case, please add them.

2. I think duplicated columns should be added to the right and not to the left, as they are right now:

   

After fix those things, this one should be ready for me.

[dpturibio]

Hello @ccordoba12, thank you for your revision.
I applied the modification to duplicate columns in the right side.
regarding commit d8264db, "supporting multiple indexes when duplicating/adding rows/cols", it is not exactly what you understood. Let me explain the behavior:
if more than one column is selected, the duplicate button is disabled. But if, for some reason we did not disable this button in the code, and more than 1 column was selected, just the last selected column would be duplicated!
Regarding the mentioned commit, it was to solve the problem of when you have a table with multiIndexes(that I didn't predicted before) as in the image:

It wasn't duplicating correctly indexes with more than on item before this commit.

[ccordoba12]

I applied the modification to duplicate columns in the right side.

Thanks! I tested it locally and it's working as expected.

regarding commit d8264db, "supporting multiple indexes when duplicating/adding rows/cols", it is not exactly what you understood

Ok, thanks for the clarification. Then I'd say this is ready.

@dpizetta, do you have additional comments? If I don't hear from you in a couple of days, I'll merge.

[dpizetta]

@ccordoba12 and @dpturibio it seems great to me. Thanks for both of you

[ccordoba12]

Fantastic contribution @dpturibio!! Thanks a lot for it!

[dpturibio]

Thank you Cordoba. It was an amazing experience. Hope to continue contributing with Spyder's community soon.