Issue 2551120
Created on 2021-03-25 01:06 by ngaba, last changed 2022-05-30 21:27 by rouilj.
msg7139 |
Author: [hidden] (ngaba) |
Date: 2021-03-25 01:06 |
|
Everything is said in the title, and it is discussed in the ML:
https://sourceforge.net/p/roundup/mailman/message/37247243/
The obtained error is:
<class 'TypeError'>: '<' not supported between instances of 'str' and 'NoneType'
|
msg7143 |
Author: [hidden] (rouilj) |
Date: 2021-03-28 02:36 |
|
Hi Nagy:
Is this the error you get when you are ordering a multilink by
an integer property?
-- rouilj
|
msg7144 |
Author: [hidden] (rouilj) |
Date: 2021-03-28 03:39 |
|
I think the way to handle this is to return tuples to
the sort routine. E.G.:
sorted(propname, reverse=False, NoneFirst=False):
NoneCode = (2,0)[NoneFirst] # use 2 if NoneFirst is False to sort
to end
def keyfunc(a):
if v:
return (1, v)
elif v is None
return (NoneCode, v)
value.sort(key=keyfunc, reverse=reverse)
Returning a tuple from the keyfunc will allow anything that is not
None to sort first by 1 and then by the value of the item. Items that
are None will sort before or after the items starting with 1 (0 or 2)
followed by comparing None to None which is supported.
E.G. sorting:
id: 1 2, 3, 4, 5
prop val: 5, 20, None, 16, None
should result in (with NoneFirst = False): 1,4,2,3,5. Sort is stable
so you should not be able to get 1,4,2,5,3. If this isn't true then I
don't see any way to add an order element last in the tuple to
preserve order.
Comments?
Also I may need to change the sort_repr methods to use the same
mechanism to sort None items as well.
|
msg7145 |
Author: [hidden] (rouilj) |
Date: 2021-03-28 14:53 |
|
I think this patch fixes the exception thrown when the sort
field includes None values. It may also fix the problem with
integers not sorting as integers.
Nagy, can you apply this (to roundup/cgi/templating.py)
and see if it fixes both this issue and issue2551122.
It also includes (commented out) code to address issue255112
by using the sort_repr function from the hyperdb base class
for the data type to sort by the base type of the data. I don't
think this is needed as using the _value from the HTMLProperty
object seems to make sorting correct.
Ralf it looks like the existing test code for test_templating is
not suitable for testing this. I think I need a real database
underneath it. Thoughts on how to test this.
-- rouilj
|
msg7152 |
Author: [hidden] (ngaba) |
Date: 2021-03-29 12:18 |
|
Dear John,
"Is this the error you get when you are ordering a multilink by
an integer property?"
When the property is itself and integer, for example on
context.documents.sorted('intprop')
where 'intprop' is a property defined as intprop = Integer() in the class 'documents' pointing to, then I get (before applying any of your patches):
<class 'TypeError'>: '<' not supported between instances of 'int' and 'NoneType'.
However, if I I call
context.documents.sorted('classprop')
where 'classprop' is a Class() with integer order property, then I still get
<class 'TypeError'>: '<' not supported between instances of 'NoneType' and 'str'.
I hope this agrees with your observations.
|
msg7153 |
Author: [hidden] (ngaba) |
Date: 2021-03-29 12:53 |
|
Now I applied your sort_within_multilink_support_NoneType.patch.
This issue2551120 is fixed (in my usecase, at least :) for both 'intprop' and 'classprop' examples, NoneFirst also works as desired.
As I see, issue2551122 is only fixed for the 'intprop' case above.
In the 'classprop' case, the obtained list is still sorted as if the order property of 'classprop' were string.
[edited to fix issue2551122: rouilj 2021-04-12]
|
msg7190 |
Author: [hidden] (rouilj) |
Date: 2021-04-12 19:40 |
|
Hi Nagy:
You said:
> However, if I I call
> context.documents.sorted('classprop')
> where 'classprop' is a Class() with integer order property,
> then I still get
>
> <class 'TypeError'>: '<' not supported between instances of 'NoneType'
and 'str'.
by classprop do you mean classprop is a multilink to a Class?
From customizing.txt:
Select a property of the class to be the order property. The order
property is used whenever using a default sort order for the class,
e.g., when grouping or sorting class A by a link to class B in the
user interface, the order property of class B is used for sorting.
That is the only sorting that I see as mentioned in customizing.txt
that seems to sort of match what you stated.
I am having issues developing a test case for this code. I'll spend
another few hours of getting a test case put together. If I can't get
something running, I'll commit the code.
Regarding issue2551122 (sorted method of MultilinkHTMLProperty does a
string sort even if the property is an integer) still failing, I am not
sure. If my description of the 'classprop' case is correct, I'll update
and continue the discussion on that issue and close this issue.
I may need a more detailed test case for that issue as I expected
this patch would have (by accident) handled both cases.
|
msg7210 |
Author: [hidden] (ngaba) |
Date: 2021-04-22 01:00 |
|
> by classprop do you mean classprop is a multilink to a Class?
No. It is just a link.
It may be undocumented, but in this case
context.documents.sorted('classprop')
sorts the documents list by the order property of the (linked) classprop property of the elements in this list. [*]
And in my opinion, this is the only sensible definition of the above command (in real life situations 'classprop' can be 'importance', for example: and instead of hard-coded numbers we use a Class() for representing the possible values of importance). Otherwise, this operation should be undefined. :)
And the same "bug" occurs in [*] as in issue2551122: The order property is always treated as string.
|
msg7233 |
Author: [hidden] (rouilj) |
Date: 2021-05-17 19:27 |
|
Committed patch, doc changes: changeset: 6414:3dbf1bc5e567
This is missing automated tests. I spent 3x more effort into getting
a test for this than I spent fixing the code. Finally gave up.
|
msg7543 |
Author: [hidden] (rouilj) |
Date: 2022-05-30 21:27 |
|
see also issue2551207.
|
|
Date |
User |
Action |
Args |
2022-05-30 21:27:26 | rouilj | set | messages:
+ msg7543 |
2021-05-17 19:27:37 | rouilj | set | status: open -> fixed resolution: fixed messages:
+ msg7233 |
2021-05-06 19:19:21 | rouilj | set | keywords:
+ Blocker |
2021-04-22 01:00:32 | ngaba | set | messages:
+ msg7210 |
2021-04-12 19:40:52 | rouilj | set | messages:
+ msg7190 |
2021-03-29 12:53:42 | ngaba | set | messages:
+ msg7153 |
2021-03-29 12:18:37 | ngaba | set | messages:
+ msg7152 |
2021-03-28 18:45:26 | rouilj | set | priority: high assignee: rouilj status: new -> open |
2021-03-28 14:53:41 | rouilj | set | files:
+ sort_within_multilink_support_NoneType.patch keywords:
+ patch messages:
+ msg7145 |
2021-03-28 03:39:02 | rouilj | set | messages:
+ msg7144 |
2021-03-28 02:36:54 | rouilj | set | messages:
+ msg7143 |
2021-03-27 03:41:44 | rouilj | set | nosy:
+ rouilj severity: minor -> major |
2021-03-25 08:30:19 | schlatterbeck | set | nosy:
+ schlatterbeck |
2021-03-25 01:06:33 | ngaba | create | |
|