-
-
djblets/db/tests/test_local_data_query_set.py (Diff revision 1) Col: 57 E231 missing whitespace after ','
-
djblets/db/tests/test_local_data_query_set.py (Diff revision 1) Col: 59 E231 missing whitespace after ','
LocalDataQuerySet order_by implementation
Review Request #7200 — Created April 14, 2015 and submitted
Queries or calls may want items in a LocalDataQuerySet to be ordered for usage. To help bridge the gap between LocalDataQuerySet and a Django queryset, an
order_by
function has been created to order items in the LocalDataQuerySet by the specified attributes, similar to Django queryset.
Test case for
order_by
.
Description | From | Last Updated |
---|---|---|
This should probably be a member of the LocalDataQuerySet class. The _COMPILED bit is unnecessary. |
brennie | |
Can we use ascending and descending here (instead of non-descending and non-ascending) ? |
brennie | |
This doesn't need a leading underscore. It won't be visible outside this function. |
brennie | |
Can this be written to use iteration instead of recursion? |
brennie | |
I think this should probably clone the data and sort that instead, e.g. copy = self.clone() copy._data.sort(...) return copy |
brennie | |
Col: 57 E231 missing whitespace after ',' |
reviewbot | |
Col: 59 E231 missing whitespace after ',' |
reviewbot | |
Function names should be verb phrases. How about compare ? |
brennie | |
The first line of a docstring should be written in the imperative mood. How about "Compare two elements of the … |
brennie | |
Since you are using attrs[i] and i, you can change this to use enumerate(attrs). |
brennie | |
Blank line between these. |
brennie | |
Blank line after class docstring. |
brennie |
Change Summary:
Removed the trailing comma in the arrays for
test_order_by
.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+72) |
-
Tool: PEP8 Style Checker Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py Tool: Pyflakes Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py
-
-
djblets/db/query.py (Diff revision 1) This should probably be a member of the
LocalDataQuerySet
class. The_COMPILED
bit is unnecessary. -
djblets/db/query.py (Diff revision 1) Can we use ascending and descending here (instead of non-descending and non-ascending) ?
-
djblets/db/query.py (Diff revision 1) This doesn't need a leading underscore. It won't be visible outside this function.
-
-
djblets/db/query.py (Diff revision 1) I think this should probably clone the data and sort that instead, e.g.
copy = self.clone() copy._data.sort(...) return copy
Change Summary:
Changed recursive call to be iterative instead.
Moved global variable as a member of a class instead.
Updated the description to be ascending and descending instead of the "non" versions.
Removed leading underscore for function naming.
Changed xrange to range for Python3 compatability.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+71) |
-
Tool: PEP8 Style Checker Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py Tool: Pyflakes Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py
-
-
-
djblets/db/query.py (Diff revision 3) The first line of a docstring should be written in the imperative mood. How about "Compare two elements of the query set"
-
djblets/db/query.py (Diff revision 3) Since you are using
attrs[i]
andi
, you can change this to useenumerate(attrs)
. -
Change Summary:
Updated to documentation and naming of the comparator function.
Used iterator for attributes instead of indexing the attributes.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+72) |
-
Tool: Pyflakes Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py Tool: PEP8 Style Checker Processed Files: djblets/db/query.py djblets/db/tests/test_local_data_query_set.py
Change Summary:
Added blank line and removed the
start
(default is already 0) argument for range in the test cases.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+73) |