PR: Enable sorting by values in Variable Explorer

[Prikers]

Fixes #5294
Fixes #5232

[pep8speaks]

Hello @Prikers! Thanks for updating the PR.

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

Line 28:1: E302 expected 2 blank lines, found 1

Comment last updated on September 23, 2017 at 12:51 Hours UTC

[goanpeca]

Hi @Prikers thanks for working on this :-)

[Prikers]

Maybe I should have done that against 3.x though 😞

[goanpeca]

You can fix it if that is the case (no need to close this PR), no worries, lets check with @ccordoba12 to see what he thinks?

[ccordoba12]

@Prikers, please read our Contributing guide to see how to move your branch to 3.x:

https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md

[Prikers]

@ccordoba12 ok, will do!
However I cannot figure out why the test is failing, it seems unrelated to this PR

[ccordoba12]

@Prikers, rebasing against the latest 3.x should fix the error in Circle.

[ccordoba12]

@Prikers, what would happen now if, as part of a list or a dict, there's a Numpy array or a Dataframe? I mean, in that case sorting should be disabled, right?

[Prikers]

@ccordoba12 nothing happens when trying to sort objects that cannot be ordered. Here are some tests:

import pandas as pd
import numpy as np

df = pd.DataFrame({'A': [1, 2, 3], 'B': [3, 2, 1]})
arr = np.random.random((10, 10))

normal_list = [1, 4, 5, 8, 9, 1, 3, 2]
obj_list = [df, arr, 4, 5, 'A']
serie = df.A

obj_dict = {'df': df, 'arr': arr, 'int': 1, 'str': 'A'}
normal_dict = {'A': 1, 'B': 2, 'C': 4, 'D': 3}
mixt_dict = {'A': '1', 'B': 2, 'C': 4, 'D': 3}

It is the same behaviour as for size for example. Is that the expected behavior? Or should we actually make the header not clickable at all? (maybe in another PR then)

[ccordoba12]

Is that the expected behavior?

Yes, it is. I just wanted to know if your changes are not introducing an error in that case.

[goanpeca]

@Prikers please check gitter :-)

[Prikers]

I am sorry guys, I will need your help once again. I had rebased this branch to 3.x and it worked. But then I pushed two more commits (fix for #5232 and testing) and now it fails. I do not understand why github thinks I am trying to merge with master... (Prikers wants to merge 4 commits into spyder-ide:master). I am obviously missing something...

[jitseniesen]

Maybe the issue is that (perhaps in addition to git rebase), you also need to rebase on the github website. Press the "Edit" button on the top of this page, next to the title of the PR, and then change the base branch and hope that all goes well (if not, it may be easier to start afresh). See https://help.github.com/articles/changing-the-base-branch-of-a-pull-request/

[Prikers]

Thanks @jitseniesen! I tried to close the PR and open it again but it does not seem to force the rebuild of continuous integration tests.
EDIT: actually it did 😄

[ccordoba12]

Looks good to me, thanks @Prikers!