-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for randomized ordering of fakequeryset #187
Add support for randomized ordering of fakequeryset #187
Conversation
I am wondering how do we test this behaviour? I thought of something like this, where we run the query n number of times, and if the ordering is different from what was seen previously, the test passes.
But I am not sure if this is the best way to do this |
It's probably fine to just run it once and verify that it contains the expected items - since that's the only thing you can rely on when using it in the real world. |
fb2a369
to
1e7c3c9
Compare
modelcluster/utils.py
Outdated
@@ -128,6 +129,8 @@ def sort_by_fields(items, fields): | |||
def get_sort_value(item): | |||
# Use a tuple of (v is not None, v) as the key, to ensure that None sorts before other values, | |||
# as comparing directly with None breaks on python3 | |||
if key == "?": # Random ordering | |||
return (True, random()) | |||
value = extract_field_value(item, key, pk_only=True, suppress_fielddoesnotexist=True) | |||
return (value is not None, value) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to use random.shuffle in this case, and avoid going through items.sort
/ get_sort_value
entirely:
if key == "?": # Random ordering
random.shuffle(items)
else:
def get_sort_value(item):
# ....
items.sort(key=get_sort_value, reverse=reverse)
Fixes #111
This PR randomizes the queryset by using
random.random()
values as the key during sorting of the results.