openlp-core team mailing list archive

[Chris Hill <chris@xxxxxxxxxxxx>]

Chris Hill has proposed merging lp:~minkus/openlp/naturalsortfix into lp:openlp.

Commit message:
Natural sort in Edit Song, Song Maintenance form; performance improvements in sorting when performing various searches

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~minkus/openlp/naturalsortfix/+merge/291024

Natural sort for authors, topics, songbooks, themes in Edit Song comboboxen & Song Maintenance form, as mentioned previously by Raoul - Matthew 21:28–29! :)

Also some minor fixes that I picked up after merge 2637 for natural sorting:
x Author, Topics, Theme search were performing two sorts (one database sort, then a natural sort in code). Removed the first database sort.
x Changed all sorting to use in-place sort() rather than copy sorted(), as apparently there's a speed bonus (http://stackoverflow.com/questions/22442378/what-is-the-difference-between-sortedlist-vs-list-sort-python)

--------------------------------
lp:~minkus/openlp/naturalsortfix (revision 2639)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1395/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1313/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1252/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1076/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/667/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/734/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/602/
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~minkus/openlp/naturalsortfix into lp:openlp.

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2016-01-30 20:41:22 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-04-05 18:07:03 +0000
@@ -34,6 +34,7 @@
 from openlp.core.common import Registry, RegistryProperties, AppLocation, UiStrings, check_directory_exists, translate
 from openlp.core.lib import FileDialog, PluginStatus, MediaType, create_separated_list
 from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box
+from openlp.core.utils import get_natural_key
 from openlp.plugins.songs.lib import VerseType, clean_song
 from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile, SongBookEntry
 from openlp.plugins.songs.lib.ui import SongStrings
@@ -110,7 +111,8 @@
         """
         Generically load a set of objects into a cache and a combobox.
         """
-        objects = self.manager.get_all_objects(cls, order_by_ref=cls.name)
+        objects = self.manager.get_all_objects(cls)
+        objects.sort(key=lambda object: get_natural_key(object.name))
         combo.clear()
         combo.addItem('')
         for obj in objects:
@@ -343,7 +345,8 @@
         """
         Load the authors from the database into the combobox.
         """
-        authors = self.manager.get_all_objects(Author, order_by_ref=Author.display_name)
+        authors = self.manager.get_all_objects(Author)
+        authors.sort(key=lambda author: get_natural_key(author.display_name))
         self.authors_combo_box.clear()
         self.authors_combo_box.addItem('')
         self.authors = []
@@ -381,6 +384,7 @@
         self.theme_combo_box.clear()
         self.theme_combo_box.addItem('')
         self.themes = theme_list
+        self.themes.sort(key=lambda theme: get_natural_key(theme))
         self.theme_combo_box.addItems(theme_list)
         set_case_insensitive_completer(self.themes, self.theme_combo_box)
 

=== modified file 'openlp/plugins/songs/forms/songmaintenanceform.py'
--- openlp/plugins/songs/forms/songmaintenanceform.py	2016-01-09 16:26:14 +0000
+++ openlp/plugins/songs/forms/songmaintenanceform.py	2016-04-05 18:07:03 +0000
@@ -27,6 +27,7 @@
 
 from openlp.core.common import Registry, RegistryProperties, UiStrings, translate
 from openlp.core.lib.ui import critical_error_message_box
+from openlp.core.utils import get_natural_key
 from openlp.plugins.songs.forms.authorsform import AuthorsForm
 from openlp.plugins.songs.forms.topicsform import TopicsForm
 from openlp.plugins.songs.forms.songbookform import SongBookForm
@@ -121,7 +122,8 @@
         Reloads the Authors list.
         """
         self.authors_list_widget.clear()
-        authors = self.manager.get_all_objects(Author, order_by_ref=Author.display_name)
+        authors = self.manager.get_all_objects(Author)
+        authors.sort(key=lambda author: get_natural_key(author.display_name))
         for author in authors:
             if author.display_name:
                 author_name = QtWidgets.QListWidgetItem(author.display_name)
@@ -135,7 +137,8 @@
         Reloads the Topics list.
         """
         self.topics_list_widget.clear()
-        topics = self.manager.get_all_objects(Topic, order_by_ref=Topic.name)
+        topics = self.manager.get_all_objects(Topic)
+        topics.sort(key=lambda topic: get_natural_key(topic.name))
         for topic in topics:
             topic_name = QtWidgets.QListWidgetItem(topic.name)
             topic_name.setData(QtCore.Qt.UserRole, topic.id)
@@ -146,7 +149,8 @@
         Reloads the Books list.
         """
         self.song_books_list_widget.clear()
-        books = self.manager.get_all_objects(Book, order_by_ref=Book.name)
+        books = self.manager.get_all_objects(Book)
+        books.sort(key=lambda book: get_natural_key(book.name))
         for book in books:
             book_name = QtWidgets.QListWidgetItem('%s (%s)' % (book.name, book.publisher))
             book_name.setData(QtCore.Qt.UserRole, book.id)

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2016-04-03 11:14:17 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2016-04-05 18:07:03 +0000
@@ -194,13 +194,13 @@
             log.debug('Authors Search')
             search_string = '%' + search_keywords + '%'
             search_results = self.plugin.manager.get_all_objects(
-                Author, Author.display_name.like(search_string), Author.display_name.asc())
+                Author, Author.display_name.like(search_string))
             self.display_results_author(search_results)
         elif search_type == SongSearch.Topics:
             log.debug('Topics Search')
             search_string = '%' + search_keywords + '%'
             search_results = self.plugin.manager.get_all_objects(
-                Topic, Topic.name.like(search_string), Topic.name.asc())
+                Topic, Topic.name.like(search_string))
             self.display_results_topic(search_results)
         elif search_type == SongSearch.Books:
             log.debug('Songbook Search')
@@ -215,7 +215,7 @@
             log.debug('Theme Search')
             search_string = '%' + search_keywords + '%'
             search_results = self.plugin.manager.get_all_objects(
-                Song, Song.theme_name.like(search_string), Song.theme_name.asc())
+                Song, Song.theme_name.like(search_string))
             self.display_results_themes(search_results)
         elif search_type == SongSearch.Copyright:
             log.debug('Copyright Search')
@@ -285,10 +285,10 @@
         """
         log.debug('display results Author')
         self.list_view.clear()
-        search_results = sorted(search_results, key=lambda author: get_natural_key(author.display_name))
+        search_results.sort(key=lambda author: get_natural_key(author.display_name))
         for author in search_results:
-            songs = sorted(author.songs, key=lambda song: song.sort_key)
-            for song in songs:
+            author.songs.sort(key=lambda song: song.sort_key)
+            for song in author.songs:
                 # Do not display temporary songs
                 if song.temporary:
                     continue
@@ -306,8 +306,8 @@
         """
         log.debug('display results Book')
         self.list_view.clear()
-        search_results = sorted(search_results, key=lambda songbook_entry:
-                                (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry)))
+        search_results.sort(key=lambda songbook_entry:
+                            (get_natural_key(songbook_entry.songbook.name), get_natural_key(songbook_entry.entry)))
         for songbook_entry in search_results:
             if songbook_entry.song.temporary:
                 continue
@@ -325,10 +325,10 @@
         """
         log.debug('display results Topic')
         self.list_view.clear()
-        search_results = sorted(search_results, key=lambda topic: get_natural_key(topic.name))
+        search_results.sort(key=lambda topic: get_natural_key(topic.name))
         for topic in search_results:
-            songs = sorted(topic.songs, key=lambda song: song.sort_key)
-            for song in songs:
+            topic.songs.sort(key=lambda song: song.sort_key)
+            for song in topic.songs:
                 # Do not display temporary songs
                 if song.temporary:
                     continue
@@ -346,8 +346,8 @@
         """
         log.debug('display results Themes')
         self.list_view.clear()
-        search_results = sorted(search_results, key=lambda song: (get_natural_key(song.theme_name),
-                                song.sort_key))
+        search_results.sort(key=lambda song: (get_natural_key(song.theme_name),
+                            song.sort_key))
         for song in search_results:
             # Do not display temporary songs
             if song.temporary:
@@ -366,9 +366,9 @@
         """
         log.debug('display results CCLI number')
         self.list_view.clear()
-        songs = sorted(search_results, key=lambda song: (get_natural_key(song.ccli_number),
-                       song.sort_key))
-        for song in songs:
+        search_results.sort(key=lambda song: (get_natural_key(song.ccli_number),
+                            song.sort_key))
+        for song in search_results:
             # Do not display temporary songs
             if song.temporary:
                 continue