Re: [Roundup-users] Some questions about file handling + some other things

[John P. R. <ro...@cs...>]

Hi Ralf:

In message <202...@ru...>,
Ralf Schlatterbeck writes:
>On Tue, Mar 23, 2021 at 11:13:20PM -0400, John P. Rouillard wrote:
>> Internally sorted uses this call to sort the data:
>> 
>>   value.sort(key=lambda a:a[property], reverse=reverse)
>> 
>> I think a[property] is always a string. Changing the code to
>> something like:
>> 
>>   cls = self._db.getclass(self._prop.classname)
>>   to_raw = cls.property.sort_repr # (or maybe cls[property])
>>   value.sort(key=lambda a:to_raw(cls, a[property], property),
>>              reverse=reverse)
>> 
>> Ralf, Bern et. al. does the derivation of to_raw to get the hyperdb
>> Integer property's sort_repr method look right?
>
>Looks good (without having tested anything) but I'm not sure to_raw is
>the correct method to use here in sorting.

I don't understand. sort_repr is the method I am using. to_raw is
just a (poorly??) chosen variable name.

>> Also it looks like this might not work as the sort_repr for the
>> hyperdb.py::Integer class just returns "val" rather than int(val).  So
>> it looks like both the Number and Integer classes need a new
>> 
>>    sort_repr(self, cls, val, name)
>> 
>> method returning respectively float(val) and int(val) (in a try block
>> to handle undefined values). This issue is definitely worth a bug
>> report. Nagy can you create a report at: issues.roundup-tracker.org
>> with the python command you tried and what you saw (basically a
>> reworded copy of your paragraph above).
>
>It is also worth a regression test: If sorting of numbers and/or dates
>works in SQL (for everything but Multilink properties) I'm quite sure
>that it would break for anydbm which always sorts in python.

Yes, Probably needs more data in the db to actually test the ordering
across multiple items. Anydbm I'll bet is currently broken since it
would use a string sort order not an integer/number sort order in its
sorting calls.

>> Going back to the question of where the null value should sort.
>> 
>> The default_value property is defined for all hyperdb types.  It is
>> referenced in cgi/templating.py (with a call to get_default_value()).
>> I don't know exactly where it shows up. The default_value AFAIK
>> doesn't show up in the db (to replace null). It may only be present in
>> the html interface or accessible by python code (e.g. an auditor).  I
>> wonder if default_value could be used somehow to control sorting
>> order?
>
>Hmm, using default_value would be too surprising when it influences the
>sort order.

Yeah. Using default_value was a attempt to put it to good use. I don't
really see why default_value was added and what it influences. It's
not (AFAICT) used to replace a None value in the database for the
property.

>I'd put this into the declaration of the property (similar
>to 'do_journal' for Link/Multilink properties. But this would mean it
>cannot be changed by the user in the index template input.

I make the same argument about this being inflexible.

>A name I'd suggest would be 'sort_null_last' which by default is 'no'.

>From an implementation point of view, sorting null last requires a
value to replace null/None so sort(key=...) doesn't throw an error
trying to sort None against any other type. Without knowing something
about the property's possible values I don't see how the code can
choose a suitable value.

>> to make any undefined value sort at the end. Changing to 0 (assuming
>> range is 0-9999)makes it sort at front. Similarly for a string: a
>> default_value of '' sorts at one end and default_value of
>> 'zzzzzzzzzzzzzzzzzz' sorts at the other end.
>
>.. until 'zzzzzzzzzzzzzzzzzzz' comes along .-)

Thank you for proving my point. The code can't know what is a valid
last/first value. The user/admin knows that an auditor prevents the
'zzzzzzzzzzzzzzzzzzz' value because the property is limited to 19
characters in length.

>> The downside of this is that you can't change it. It's hard-coded in
>> the schema. (Well you can change __default_value but no don't do
>> that.)

As you also noted this is inflexible.

>> Maybe better to make:
>> 
>>    sorted(self, property, reverse=False, nullval=None)
>
>Hmm, I would make this a boolean property (NULL sorted either in front
>or to the back), this would also make it work for all values, you cannot
>find a "nullval" value in the example above that works across all data.
>And I do not see a use-case where you want NULL-values to be sorted in
>the middle somewhere.

Consider a multilink called surveys with a 'rating' property. We want
to display the links in rating order. 'rating' has values [-5,5] where
0 is neutral. You want "no rating given"/unset to sort as 0. I am sure
there are other examples. Note if 'rating' had values [0,10] then 'no
rating' should be 5. If you wanted all 'no value given' to sort at the
end, you choose 11 as the 'no rating given" value.

>If we really need this to be specified by the user, which I doubt (note
>that the filter method already has a lot of parameters that are
>currently not exposed to html, I'm reluctant at this point to add more).

I think we have no choice but to have the user set this value. The
code has to choose a value to keep using the sort() function. As shown
above there is no way for the code to provide a value that makes
sense.

>Currently sort/group parameters to filter are specified as lists of
>tuples of the sort direction and the property name (I always forget
>which one comes first, just looked it up, the direction is first). We
>*could* add another character to the direction, e.g. '~' to specify the
>handling of NULL values. Something like 
>
>db.someclass.filter (None, filterspec, sort = [('+~', propname)]
>But this is ugly :-)
>Or an optional third parameter sort = [('+', propname, 'null-last')]

Up till this point I have been considering only the sorted() method
for MultilinkHTMLProperty in cgi/templating.py. However the backends
use sort_repr as well. So your comment above about adding it to the
property declaration seems like it is needed as well. So the cascade
for sorted() would be:

     if val is None:
        if nullval is None: # use the order declared in the property
           return cls.property.get_null_value # (to be created)

names to be determined. (Gee naming things is tough.)

This provides:

    a default order associated with the property so multilink sorting
    in the backends works.

    the ability to display in a different ordering in the template by
    choosing an approprate value.

Thoughts?

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.