GregorCH / ipet

Interactive Performance Evaluation Tools for Optimization Software
MIT License
26 stars 6 forks source link

Use sortlevel argument in evaluation #28

Open fschloesser opened 7 years ago

fschloesser commented 7 years ago

Currently 'sortlevel' it is set but not used.

fschloesser commented 7 years ago

What exactly is this option supposed to do?

GregorCH commented 7 years ago

Have you read the documentation of sortlevel?

sortlevel : level on which to base column sorting, '0' for group level, '1' for column level
fschloesser commented 7 years ago

I have, but i don't get it. And group level is not even used anymore.

GregorCH commented 7 years ago

The column headers consist of >= 1 levels, depending on attributes such as indexsplit. The sortlevel functionality should allow to specify the level of the column keys, by which the resulting long table should be sorted. Implicitly, we currently use a sortlevel of 1, I guess.

GregorCH commented 7 years ago

You are right, group level is too specific and has been generalized in the mean time (because multiple levels of group keys are possible).

fschloesser commented 7 years ago

Thank you. So it should just be a pandas call to some kind of sortlevel func in a few lines, right? Does this apply to both aggregated and long table?

GregorCH commented 7 years ago

Yes, there is a pandas function, (sort_axis, I guess). We are free to do whatever we want. I would suggest that it applies to both data frames, although the meaning would be a bit different for the aggregated table, because the column keys are used for the rows, as well.

GregorCH commented 7 years ago

There should be, IMHO, the possibilty not to sort, maybe by assigning None to the sortlevel (Note that this should be even the default behavior).

fschloesser commented 6 years ago

I have dealt with this in 0d5bd2ca19f03abf2bfcb59827398654ce158570

GregorCH commented 6 years ago

Why did you decide to push this directly instead of creating a merge request?

fschloesser commented 6 years ago

Because it was a simple thing (i thought). And in this project simple things seem to almost never be done via a pull request.