LocalDataQuerySet order_by implementation

Review Request #7200 — Created April 14, 2015 and submitted

Information

Djblets
master
c895f39...

Reviewers

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.

brenniebrennie

Can we use ascending and descending here (instead of non-descending and non-ascending) ?

brenniebrennie

This doesn't need a leading underscore. It won't be visible outside this function.

brenniebrennie

Can this be written to use iteration instead of recursion?

brenniebrennie

I think this should probably clone the data and sort that instead, e.g. copy = self.clone() copy._data.sort(...) return copy

brenniebrennie

Col: 57 E231 missing whitespace after ','

reviewbotreviewbot

Col: 59 E231 missing whitespace after ','

reviewbotreviewbot

Function names should be verb phrases. How about compare ?

brenniebrennie

The first line of a docstring should be written in the imperative mood. How about "Compare two elements of the …

brenniebrennie

Since you are using attrs[i] and i, you can change this to use enumerate(attrs).

brenniebrennie

Blank line between these.

brenniebrennie

Blank line after class docstring.

brenniebrennie
reviewbot
  1. 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
    
    
  2. Show all issues
    Col: 57
     E231 missing whitespace after ','
    
  3. Show all issues
    Col: 59
     E231 missing whitespace after ','
    
  4. 
      
VT
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. djblets/db/query.py (Diff revision 1)
     
     
    Show all issues

    This should probably be a member of the LocalDataQuerySet class. The _COMPILED bit is unnecessary.

  3. djblets/db/query.py (Diff revision 1)
     
     
     
    Show all issues

    Can we use ascending and descending here (instead of non-descending and non-ascending) ?

  4. djblets/db/query.py (Diff revision 1)
     
     
    Show all issues

    This doesn't need a leading underscore. It won't be visible outside this function.

  5. djblets/db/query.py (Diff revision 1)
     
     
     
    Show all issues

    Can this be written to use iteration instead of recursion?

  6. djblets/db/query.py (Diff revision 1)
     
     
     
     
    Show all issues

    I think this should probably clone the data and sort that instead, e.g.

    copy = self.clone()
    copy._data.sort(...)
    return copy
    
  7. 
      
VT
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. djblets/db/query.py (Diff revision 3)
     
     
    Show all issues

    Function names should be verb phrases. How about compare ?

  3. djblets/db/query.py (Diff revision 3)
     
     
    Show all issues

    The first line of a docstring should be written in the imperative mood. How about "Compare two elements of the query set"

  4. djblets/db/query.py (Diff revision 3)
     
     
    Show all issues

    Since you are using attrs[i] and i, you can change this to use enumerate(attrs).

  5. djblets/db/query.py (Diff revision 3)
     
     
     
    Show all issues

    Blank line between these.

  6. 
      
VT
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. Ship It!
  2. 
      
brennie
  1. 
      
    1. Sorry, I missed one thing. After this is fixed, I'll land it.

  2. djblets/db/query.py (Diff revision 4)
     
     
     
    Show all issues

    Blank line after class docstring.

  3. 
      
VT
reviewbot
  1. 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
    
    
  2. 
      
VT
Review request changed
Status:
Completed
Change Summary:
Pushed to master (ba64942)