2.1.0-2.fc33 :: size sorting is broken

[adriansev]

Hi! In the latest available version of dnf-dragora the size sorting is bogus e.g. : descending order i get : 9994.2k 999.7k 998.5k 996.6k 994.9k 9928.2k ...

[anaselli]

There is no particular sorting algorithm. What you see it is probably a string (ascii) sort

[adriansev]

well, then it's a bug.. the algorithm of sort -h should be used instead

[anaselli]

have you have a patch in mind for that?

[adriansev]

i have absolutely no familiarity with this code base, and worse, no familiarity with UI interfaces and graphic concepts. But if you could point to me where is the sorting done i can try to take a stab at it (i imagine that is a sorting of a list with objects, so i might interfere with the sorting...) to run it, i can just simply use the dnfdragora from bin?

[anaselli]

What i meant in my previous comment is that libyui does not offer an algorithm to sort a column in the version we're using. But I recently found this what i need to understand is how it works and of course if it does what you expected using libyio-qt then try to add the related code to libyui-gtk. Though I'm afraid there is no events for column sorting, so there is no way to manage a different kind of sorting but the default one (that is the one we're using now).

[adriansev]

isn't it possible to add just to dnfdragora a sorting for sizes? if you can point me to a container with packages that is used in representation i can add a sorting function just for sizes (really simple one, like converting units to corresponding ints, sort numerically and return the sorted list )

[anaselli]

To do something in dnfdragora we should block the sorting attribute of YCBTable and then adding another way to provide sorting, for instance by adding push buttons or checkboxes (or radio buttons), or a sorting menu in which choosing the way in which data are presented (statically). Sorting is pretty well managed in python, moreover we have package attribute not as strings to represent size, so size is size with the same units for all the packages. But i think that is a bit ugly from the user point of view, no?

Perhaps a right way should be to add something in libyui to manage the click on table header event that normally would be a default sorting method, in such a way items could be ordered in a custom way, but it is hard to get that i believe...

[adriansev]

so.. it seems that it's really hard for me to offer any code, as i have to get to know all internals of both dnfdragora and libyui .. but, if you are correct and all size strings have 'K' suffix then, the solution would be just to, for Size column, strip the K from string, do numerical sort, and then put the K back ... i still do not understand if the bug should be filled to libyui or here.. my reason to write here is that libyui is just a library, the way it is used is another matter... So, to reiterate: the dnfdragora sorting of packages by size is broken (the sorting is NOT by size). Is the bug part of dnfdragora or from libyui? also, it's clear that i cannot help with integrating any code when i know nothing about all the dependencies and ecosystem, but if someone can provide me with an example list/dict that have a size element i can provide back a way to sort the size numerically ..
Thank you!

[anaselli]

Well removing the "K" is not enough since string sorting does not help anyway (look at the image). I wrote a C++ example that uses YTable in a simple way, let's say if you're able to make that working i can change dnfdragora accordingly :)

I can also give a python example if you need or point you to dnfdragora code but as you can see in the example i haven't managed any sorting i just added items.

and what is fun is that Gtk one below is different

[adriansev]

@anaselli quick question: how can you recover something that you put into YTable? Because you add YTableItem (s) but when iterating over the table, the iterator gives you YItem* that cannot be up-casted to YTableItem* .. so, the current status is this https://gist.github.com/adriansev/0024d4ded65de8914d3243c756eb4ea8 Any idea how can i replace the method label(int x) of YTableItem with something to work on YItem? should i iterate over YItem with childrenBegin/End and just stop at 3rd child? (and this would/should be the equivalent of label(s) )

[anaselli]

Well you can use dynamic_cast for instance, and you can use table->deleteAllItems() to remove all items and insert new ones again. But it isn't clear to me what you wanted to do. The problem is not how to set items into table when i add them i can choose any sorting method i want, maybe for name and size for instance, but once package list is inserted then user can press the column name and that changes, you can fix it by using setKeepSorting but then you should add something like this example for any columns, and i think that is pretty ugly... but working :)

And Gtk next, (but i found anoter gtk issue... column headers are still clickable for sorting there)

P.S. you could reach us at IRC freenode channel #manatools to talk if you want

[adriansev]

exactly, that's what i wanted to do, but it was not clear to me where is the source of the objects to be inserted in the table, and i was trying to get the items from table, to be sorted and inserted in the table again. From my point of view you succeeded to do it .. 2 comments i would have:

1. you can control the width of the string with stringstream:

root [0] float num_float = 123.4567F;
root [1] std::stringstream ss1;
root [2] ss1 << std::fixed << std::setprecision(2) << num_float;
root [3] cout << ss1.str() << endl;
123.46

2. i'm particular happy that you put the units in the column header (as they should). But regarding the notation, arguments can be given that JEDEC is not such a powerful standardisation body, so one could choose IEC/ISQ designation, meaning KiB for 1024 bytes (while the metric designation is kB for 1000 bytes). I would like to bring the argument that KiB would be clear without any doubt what it means, whereas KB would not (moreover the notations for MB and GB are the same for metric version so, if used, confusion can arise.)

I will take a more detailed look at your example so see if i can bring some value, but already looks good. Thanks a lot!!

[anaselli]

Though i don't like to have buttons for sorting. That should mean having a button for any columns...

[adriansev]

you mean, a different button other than what is clickable on the header? cannot the sorting be integrated there?

[anaselli]

I mean that. To integrate sorting in the header you need to work on libyui and its plugins (libyui-qt, libyui-gtk and libyui-ncurses). Let's say we use YCBTable that is not under Suse control and it is defined in libyui-mga (and its plugin definitions libyui-mga-qt, libyui-mga-gtk and libyui-mga-ncurses) so yes we can change something there, but that requires a lot of reworking, though i'm planning to better extend YCBTable from Ytable in the near future that could be a moment to thought to it... Anyway you have data now to try working on it, i won't stop you if you find a good way i will consider it :)

[adriansev]

sure, i will get back to you as soon as i have something .. it will take some time though as i have a lot of source reading to do :)

[ntm]

Hello, sorting on size is still broken with dnfdragora-2.1.2-3.fc37. That's unfortunate, I think that feature would be very useful. It seems the ideas discussed in Dec 2020 didn't lead to a solution. I understand from above discussions that the issue stems from the underlying library libyui only sorting by alphanumeric order rather than numeric, and you were trying to work around this and get a numeric sort to work (but with a differint UI, clicking on column headers wouldn't work). Here's another idea: find the largest rpm size (in KB), then left-pad every rpm size with leading 0's so that all sizes are represented as strings with the same number of digits (ignoring the decimals). The user can then use the standard libyui alphanumeric string sort on this column and the result is actually a numeric sort. In your example the sizes would be: 2430 1234.5 0100 0012 0008.2 0003 This will get sorted numerically when applying a string sort. Seems reasonable?

[anaselli]

i think having a package size of 0003 is pretty ugly :D. Though you could try to play here https://github.com/manatools/dnfdragora/blob/master/dnfdragora/ui.py#L755 go get your implementation working and propose a patch maybe. I'd prefer a better solution though, but i'm open to discuss ofcourse ;)

[ntm]

i think having a package size of 0003 is pretty ugly :D.

I was just mentioning this as a way to get an alphanumerical-string-sort to behave correctly when we want to sort numerically. But I agree, adding a new column with this content is ugly, and although it allows users to sort by size I think its ugliness makes it a poor solution.

However, I see that the underlying libyui C library YTableCell allows cells to have a sortKey field. I believe this is exactly what we want: you would use the sizes left-padded with 0's as sortKey, and then clicking on the "Size" column would sort the sizes based on this sortkey, i.e. we would get a numerical sort-by-size. https://libyui.github.io/libyui/api-doc/classYTableCell.html

Problem is I can't find any documentation for the python yui module, so I don't know if YTableCell::sortKey is exposed / accessible in the python interface.

[anaselli]

Interesting thing, sortKey i mean, i need to give it a look, and understand how it is working... BTW dnfdragora uses a YTable "fork" e.g. YMGA_CBTable from libyui-mga plugin, and the relate YCBTableItem. There we could implement something better to sort, but i don't know how to start and have no time to find a solution for all the three plugins (ncurses, gtk and Qt).

As far as Python yui concerned, it is just a swig binding to C++ libyui, so it should work the same.

[anaselli]

According to libyui-qt and libyui-ncurses compare has been implemented if sortkey is set (i need to understand how it works though yet), even if compare just converts string into long, so one like "100K" would fail anyway. However it has not been implemented in libyui-gtk, if someone has C/C++ expertise could provide a patch there to help me in fixing this. I will check how i should fix it in libyui-mga plugin eventually.

[ntm]

I've opened PR #210 , which solves this issue in my tests.

[anaselli]

Merged and added a new release 2.1.4. Thanks for the help!