Merge lp:~m2j/openlp/bug-1094342 into lp:openlp

Proposed by Meinert Jordan
Status: Merged
Approved by: Tim Bentley
Approved revision: 2234
Merged at revision: 2230
Proposed branch: lp:~m2j/openlp/bug-1094342
Merge into: lp:openlp
Diff against target: 515 lines (+97/-103)
15 files modified
openlp/core/ui/exceptionform.py (+9/-0)
openlp/core/ui/thememanager.py (+2/-2)
openlp/core/utils/__init__.py (+30/-16)
openlp/plugins/bibles/forms/bibleimportform.py (+2/-2)
openlp/plugins/bibles/lib/mediaitem.py (+3/-3)
openlp/plugins/custom/lib/db.py (+3/-4)
openlp/plugins/images/lib/mediaitem.py (+6/-6)
openlp/plugins/media/lib/mediaitem.py (+3/-3)
openlp/plugins/presentations/lib/mediaitem.py (+2/-3)
openlp/plugins/songs/forms/songexportform.py (+1/-2)
openlp/plugins/songs/lib/__init__.py (+1/-35)
openlp/plugins/songs/lib/db.py (+4/-24)
openlp/plugins/songs/lib/mediaitem.py (+2/-2)
scripts/check_dependencies.py (+1/-0)
tests/functional/openlp_core_utils/test_utils.py (+28/-1)
To merge this branch: bzr merge lp:~m2j/openlp/bug-1094342
Reviewer Review Type Date Requested Status
Tim Bentley Approve
Andreas Preikschat (community) Approve
Raoul Snyman Pending
Review via email: mp+157524@code.launchpad.net

This proposal supersedes a proposal from 2013-04-06.

Description of the change

This branch replaces the "cmp=" argument when sorting a list of strings. This is necessary for Python3 transition.

Additionally it uses ICU for locale aware string sorting. For Windows this is mandatory as Microsoft provides a broken locale.strcoll method. For Python 2 we can't use Pythons locale aware sorting, as a bug breaks the key generation for unicode.

I replaced the natural compare algorithm in openlp/plugins/songs/lib/__init__.py and placed it in openlp/core/utils/__init__.py as there might be wider use. Additional I precompute the full sorting key for song sorting. This should improve the song sorting performance significantly.

The code in lines 113 to 114 is commented out as it is not needed in Python 2. The expected performance penalty should be low, but still not necessary.

To post a comment you must log in.
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

Works on windows and one linux. I tested the sorting only with py2. On py3 I only tested if it starts.

I haven't reviewed the code nor did performance tests yet.

Revision history for this message
Meinert Jordan (m2j) wrote : Posted in a previous version of this proposal

> Works on windows and one linux. I tested the sorting only with py2. On py3 I
> only tested if it starts.
>
> I haven't reviewed the code nor did performance tests yet.

Hi, I've looked to the Song class, and realized, that it makes sence to buffer the whole key generation for song titles. This should improve the overall performance (compared to trunk). So just wait for my next code commit.

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

Can you replace this:
105 + key = re.findall(r'(\d+|\D+)', string)

by

key = REGEX_WHATEVER.findall(string)

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

92 + if ICU_COLLATOR is None:
93 + from languagemanager import LanguageManager

And this is okay.

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

I tested your branch and compared it to trunk:
branch: 0:00:00.001660
trunk: 0:00:00.008635

2961 songs

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

> I tested your branch and compared it to trunk:
> branch: 0:00:00.001660
> trunk: 0:00:00.008635
>
> 2961 songs

(Note: This does not consider any pre-computing)

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

Use list comprehension:
108 + for index, part in enumerate(key):
109 + if part.isdigit():
110 + key[index] = int(part)
111 + else:
112 + key[index] = get_local_key(part)

return [int(part) if part.isdigit() else get_local_key(part) for part in keys]

Hm.. whats with memory usage then? Will "key" be a list with all strings to sort?

Revision history for this message
Meinert Jordan (m2j) wrote : Posted in a previous version of this proposal

I will compile the regexp in a variable called DIGITS_OR_NONDIGITS.
I will use list compression. I used a loop before, as there were originally more code in the loop.

About the memory usage: It is approximately same as before. The only difference is, that we are storing keys for the strings instead of the strings themself.

What is key?:
It is a list of string keys and integers. If you compare them, it will compare the elements.
Example:
strings: sort([u'Item 2', u'Item 1b'], key=get_natural_key)
keys: sort([[u'item ', 2], [u'item ', 1, u' b']])
u'item ' == u'item '
1 < 2

So, key is a item per song, which provides __gt__(), __lt__(), and __eq__() methods for a correct sorting order.

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

You should add a test to your proposal ;)

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

9 + ICU_VERSION = u'OK'
You should be using icu.VERSION. However, I do not know if that always existed. So maybe we need another try-catch around it.

review: Needs Fixing
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

But the rest looks good :)

Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

+

review: Approve
Revision history for this message
Tim Bentley (trb143) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

93 +def get_local_key(string):

Surely that should be "get_locale_key"?

review: Needs Fixing
Revision history for this message
Meinert Jordan (m2j) wrote : Posted in a previous version of this proposal

Thanks the approves and the "locale" hint.

Revision history for this message
Raoul Snyman (raoul-snyman) wrote : Posted in a previous version of this proposal

=== added file 'tests/interfaces/openlp_plugins/__init__.pyc.OTHER'

This file needs to be removed.

review: Needs Fixing
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote : Posted in a previous version of this proposal

You missed to fix the tests (function renaming).

review: Needs Fixing
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) :
review: Approve
Revision history for this message
Tim Bentley (trb143) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/exceptionform.py'
2--- openlp/core/ui/exceptionform.py 2013-03-14 10:46:19 +0000
3+++ openlp/core/ui/exceptionform.py 2013-04-06 19:11:20 +0000
4@@ -70,6 +70,14 @@
5 except ImportError:
6 MAKO_VERSION = u'-'
7 try:
8+ import icu
9+ try:
10+ ICU_VERSION = icu.VERSION
11+ except AttributeError:
12+ ICU_VERSION = u'OK'
13+except ImportError:
14+ ICU_VERSION = u'-'
15+try:
16 import uno
17 arg = uno.createUnoStruct(u'com.sun.star.beans.PropertyValue')
18 arg.Name = u'nodepath'
19@@ -143,6 +151,7 @@
20 u'PyEnchant: %s\n' % ENCHANT_VERSION + \
21 u'PySQLite: %s\n' % SQLITE_VERSION + \
22 u'Mako: %s\n' % MAKO_VERSION + \
23+ u'pyICU: %s\n' % ICU_VERSION + \
24 u'pyUNO bridge: %s\n' % UNO_VERSION + \
25 u'VLC: %s\n' % VLC_VERSION
26 if platform.system() == u'Linux':
27
28=== modified file 'openlp/core/ui/thememanager.py'
29--- openlp/core/ui/thememanager.py 2013-03-19 19:43:22 +0000
30+++ openlp/core/ui/thememanager.py 2013-04-06 19:11:20 +0000
31@@ -44,7 +44,7 @@
32 from openlp.core.lib.ui import critical_error_message_box, create_widget_action
33 from openlp.core.theme import Theme
34 from openlp.core.ui import FileRenameForm, ThemeForm
35-from openlp.core.utils import AppLocation, delete_file, locale_compare, get_filesystem_encoding
36+from openlp.core.utils import AppLocation, delete_file, get_locale_key, get_filesystem_encoding
37
38 log = logging.getLogger(__name__)
39
40@@ -418,7 +418,7 @@
41 self.theme_list_widget.clear()
42 files = AppLocation.get_files(self.settings_section, u'.png')
43 # Sort the themes by its name considering language specific
44- files.sort(key=lambda file_name: unicode(file_name), cmp=locale_compare)
45+ files.sort(key=lambda file_name: get_locale_key(unicode(file_name)))
46 # now process the file list of png files
47 for name in files:
48 # check to see file is in theme root directory
49
50=== modified file 'openlp/core/utils/__init__.py'
51--- openlp/core/utils/__init__.py 2013-03-04 10:55:02 +0000
52+++ openlp/core/utils/__init__.py 2013-04-06 19:11:20 +0000
53@@ -38,6 +38,7 @@
54 from subprocess import Popen, PIPE
55 import sys
56 import urllib2
57+import icu
58
59 from PyQt4 import QtGui, QtCore
60
61@@ -56,10 +57,12 @@
62 log = logging.getLogger(__name__)
63 APPLICATION_VERSION = {}
64 IMAGES_FILTER = None
65+ICU_COLLATOR = None
66 UNO_CONNECTION_TYPE = u'pipe'
67 #UNO_CONNECTION_TYPE = u'socket'
68 CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE)
69 INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]', re.UNICODE)
70+DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+', re.UNICODE)
71
72
73 class VersionThread(QtCore.QThread):
74@@ -379,21 +382,32 @@
75 return re.sub('\%[a-zA-Z]', match_formatting, text)
76
77
78-def locale_compare(string1, string2):
79- """
80- Compares two strings according to the current locale settings.
81-
82- As any other compare function, returns a negative, or a positive value,
83- or 0, depending on whether string1 collates before or after string2 or
84- is equal to it. Comparison is case insensitive.
85- """
86- # Function locale.strcoll() from standard Python library does not work properly on Windows.
87- return locale.strcoll(string1.lower(), string2.lower())
88-
89-
90-# For performance reasons provide direct reference to compare function without wrapping it in another function making
91-# the string lowercase. This is needed for sorting songs.
92-locale_direct_compare = locale.strcoll
93+def get_locale_key(string):
94+ """
95+ Creates a key for case insensitive, locale aware string sorting.
96+ """
97+ string = string.lower()
98+ # For Python 3 on platforms other than Windows ICU is not necessary. In those cases locale.strxfrm(str) can be used.
99+ global ICU_COLLATOR
100+ if ICU_COLLATOR is None:
101+ from languagemanager import LanguageManager
102+ locale = LanguageManager.get_language()
103+ icu_locale = icu.Locale(locale)
104+ ICU_COLLATOR = icu.Collator.createInstance(icu_locale)
105+ return ICU_COLLATOR.getSortKey(string)
106+
107+
108+def get_natural_key(string):
109+ """
110+ Generate a key for locale aware natural string sorting.
111+ Returns a list of string compare keys and integers.
112+ """
113+ key = DIGITS_OR_NONDIGITS.findall(string)
114+ key = [int(part) if part.isdigit() else get_locale_key(part) for part in key]
115+ # Python 3 does not support comparision of different types anymore. So make sure, that we do not compare str and int.
116+ #if string[0].isdigit():
117+ # return [''] + key
118+ return key
119
120
121 from applocation import AppLocation
122@@ -403,4 +417,4 @@
123
124 __all__ = [u'AppLocation', u'ActionList', u'LanguageManager', u'get_application_version', u'check_latest_version',
125 u'add_actions', u'get_filesystem_encoding', u'get_web_page', u'get_uno_command', u'get_uno_instance',
126- u'delete_file', u'clean_filename', u'format_time', u'locale_compare', u'locale_direct_compare']
127+ u'delete_file', u'clean_filename', u'format_time', u'get_locale_key', u'get_natural_key']
128
129=== modified file 'openlp/plugins/bibles/forms/bibleimportform.py'
130--- openlp/plugins/bibles/forms/bibleimportform.py 2013-03-19 19:43:22 +0000
131+++ openlp/plugins/bibles/forms/bibleimportform.py 2013-04-06 19:11:20 +0000
132@@ -38,7 +38,7 @@
133 from openlp.core.lib.db import delete_database
134 from openlp.core.lib.ui import critical_error_message_box
135 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
136-from openlp.core.utils import AppLocation, locale_compare
137+from openlp.core.utils import AppLocation, get_locale_key
138 from openlp.plugins.bibles.lib.manager import BibleFormat
139 from openlp.plugins.bibles.lib.db import BiblesResourcesDB, clean_filename
140
141@@ -455,7 +455,7 @@
142 """
143 self.webTranslationComboBox.clear()
144 bibles = self.web_bible_list[index].keys()
145- bibles.sort(cmp=locale_compare)
146+ bibles.sort(key=get_locale_key)
147 self.webTranslationComboBox.addItems(bibles)
148
149 def onOsisBrowseButtonClicked(self):
150
151=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
152--- openlp/plugins/bibles/lib/mediaitem.py 2013-03-28 20:08:07 +0000
153+++ openlp/plugins/bibles/lib/mediaitem.py 2013-04-06 19:11:20 +0000
154@@ -36,7 +36,7 @@
155 from openlp.core.lib.searchedit import SearchEdit
156 from openlp.core.lib.ui import set_case_insensitive_completer, create_horizontal_adjusting_combo_box, \
157 critical_error_message_box, find_and_set_in_combo_box, build_icon
158-from openlp.core.utils import locale_compare
159+from openlp.core.utils import get_locale_key
160 from openlp.plugins.bibles.forms import BibleImportForm, EditBibleForm
161 from openlp.plugins.bibles.lib import LayoutStyle, DisplayStyle, VerseReferenceList, get_reference_separator, \
162 LanguageSelection, BibleStrings
163@@ -325,7 +325,7 @@
164 # Get all bibles and sort the list.
165 bibles = self.plugin.manager.get_bibles().keys()
166 bibles = filter(None, bibles)
167- bibles.sort(cmp=locale_compare)
168+ bibles.sort(key=get_locale_key)
169 # Load the bibles into the combo boxes.
170 self.quickVersionComboBox.addItems(bibles)
171 self.quickSecondComboBox.addItems(bibles)
172@@ -461,7 +461,7 @@
173 for book in book_data:
174 data = BiblesResourcesDB.get_book_by_id(book.book_reference_id)
175 books.append(data[u'name'] + u' ')
176- books.sort(cmp=locale_compare)
177+ books.sort(key=get_locale_key)
178 set_case_insensitive_completer(books, self.quickSearchEdit)
179
180 def on_import_click(self):
181
182=== modified file 'openlp/plugins/custom/lib/db.py'
183--- openlp/plugins/custom/lib/db.py 2013-01-05 22:17:30 +0000
184+++ openlp/plugins/custom/lib/db.py 2013-04-06 19:11:20 +0000
185@@ -35,7 +35,7 @@
186 from sqlalchemy.orm import mapper
187
188 from openlp.core.lib.db import BaseModel, init_db
189-from openlp.core.utils import locale_compare
190+from openlp.core.utils import get_locale_key
191
192 class CustomSlide(BaseModel):
193 """
194@@ -44,11 +44,10 @@
195 # By default sort the customs by its title considering language specific
196 # characters.
197 def __lt__(self, other):
198- r = locale_compare(self.title, other.title)
199- return True if r < 0 else False
200+ return get_locale_key(self.title) < get_locale_key(other.title)
201
202 def __eq__(self, other):
203- return 0 == locale_compare(self.title, other.title)
204+ return get_locale_key(self.title) == get_locale_key(other.title)
205
206
207 def init_schema(url):
208
209=== modified file 'openlp/plugins/images/lib/mediaitem.py'
210--- openlp/plugins/images/lib/mediaitem.py 2013-03-23 07:07:06 +0000
211+++ openlp/plugins/images/lib/mediaitem.py 2013-04-06 19:11:20 +0000
212@@ -36,7 +36,7 @@
213 StringContent, TreeWidgetWithDnD, UiStrings, build_icon, check_directory_exists, check_item_selected, \
214 create_thumb, translate, validate_thumb
215 from openlp.core.lib.ui import create_widget_action, critical_error_message_box
216-from openlp.core.utils import AppLocation, delete_file, locale_compare, get_images_filter
217+from openlp.core.utils import AppLocation, delete_file, get_locale_key, get_images_filter
218 from openlp.plugins.images.forms import AddGroupForm, ChooseGroupForm
219 from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups
220
221@@ -255,7 +255,7 @@
222 The ID of the group that will be added recursively
223 """
224 image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
225- image_groups.sort(cmp=locale_compare, key=lambda group_object: group_object.group_name)
226+ image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
227 folder_icon = build_icon(u':/images/image_group.png')
228 for image_group in image_groups:
229 group = QtGui.QTreeWidgetItem()
230@@ -286,7 +286,7 @@
231 combobox.clear()
232 combobox.top_level_group_added = False
233 image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
234- image_groups.sort(cmp=locale_compare, key=lambda group_object: group_object.group_name)
235+ image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
236 for image_group in image_groups:
237 combobox.addItem(prefix + image_group.group_name, image_group.id)
238 self.fill_groups_combobox(combobox, image_group.id, prefix + ' ')
239@@ -338,7 +338,7 @@
240 self.expand_group(open_group.id)
241 # Sort the images by its filename considering language specific
242 # characters.
243- images.sort(cmp=locale_compare, key=lambda image_object: os.path.split(unicode(image_object.filename))[1])
244+ images.sort(key=lambda image_object: get_locale_key(os.path.split(unicode(image_object.filename))[1]))
245 for imageFile in images:
246 log.debug(u'Loading image: %s', imageFile.filename)
247 filename = os.path.split(imageFile.filename)[1]
248@@ -525,9 +525,9 @@
249 group_items.append(item)
250 if isinstance(item.data(0, QtCore.Qt.UserRole), ImageFilenames):
251 image_items.append(item)
252- group_items.sort(cmp=locale_compare, key=lambda item: item.text(0))
253+ group_items.sort(key=lambda item: get_locale_key(item.text(0)))
254 target_group.addChildren(group_items)
255- image_items.sort(cmp=locale_compare, key=lambda item: item.text(0))
256+ image_items.sort(key=lambda item: get_locale_key(item.text(0)))
257 target_group.addChildren(image_items)
258
259 def generate_slide_data(self, service_item, item=None, xmlVersion=False,
260
261=== modified file 'openlp/plugins/media/lib/mediaitem.py'
262--- openlp/plugins/media/lib/mediaitem.py 2013-03-23 06:46:41 +0000
263+++ openlp/plugins/media/lib/mediaitem.py 2013-04-06 19:11:20 +0000
264@@ -37,7 +37,7 @@
265 from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box
266 from openlp.core.ui import DisplayController, Display, DisplayControllerType
267 from openlp.core.ui.media import get_media_players, set_media_players
268-from openlp.core.utils import AppLocation, locale_compare
269+from openlp.core.utils import AppLocation, get_locale_key
270
271 log = logging.getLogger(__name__)
272
273@@ -261,7 +261,7 @@
274 def load_list(self, media, target_group=None):
275 # Sort the media by its filename considering language specific
276 # characters.
277- media.sort(cmp=locale_compare, key=lambda filename: os.path.split(unicode(filename))[1])
278+ media.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1]))
279 for track in media:
280 track_info = QtCore.QFileInfo(track)
281 if not os.path.exists(track):
282@@ -287,7 +287,7 @@
283
284 def getList(self, type=MediaType.Audio):
285 media = Settings().value(self.settings_section + u'/media files')
286- media.sort(cmp=locale_compare, key=lambda filename: os.path.split(unicode(filename))[1])
287+ media.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1]))
288 ext = []
289 if type == MediaType.Audio:
290 ext = self.media_controller.audio_extensions_list
291
292=== modified file 'openlp/plugins/presentations/lib/mediaitem.py'
293--- openlp/plugins/presentations/lib/mediaitem.py 2013-03-23 06:46:41 +0000
294+++ openlp/plugins/presentations/lib/mediaitem.py 2013-04-06 19:11:20 +0000
295@@ -35,7 +35,7 @@
296 from openlp.core.lib import MediaManagerItem, Registry, ItemCapabilities, ServiceItemContext, Settings, UiStrings, \
297 build_icon, check_item_selected, create_thumb, translate, validate_thumb
298 from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box
299-from openlp.core.utils import locale_compare
300+from openlp.core.utils import get_locale_key
301 from openlp.plugins.presentations.lib import MessageListener
302
303 log = logging.getLogger(__name__)
304@@ -153,8 +153,7 @@
305 if not initialLoad:
306 self.main_window.display_progress_bar(len(files))
307 # Sort the presentations by its filename considering language specific characters.
308- files.sort(cmp=locale_compare,
309- key=lambda filename: os.path.split(unicode(filename))[1])
310+ files.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1]))
311 for file in files:
312 if not initialLoad:
313 self.main_window.increment_progress_bar()
314
315=== modified file 'openlp/plugins/songs/forms/songexportform.py'
316--- openlp/plugins/songs/forms/songexportform.py 2013-03-14 22:22:18 +0000
317+++ openlp/plugins/songs/forms/songexportform.py 2013-04-06 19:11:20 +0000
318@@ -37,7 +37,6 @@
319 from openlp.core.lib import Registry, UiStrings, create_separated_list, build_icon, translate
320 from openlp.core.lib.ui import critical_error_message_box
321 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
322-from openlp.plugins.songs.lib import natcmp
323 from openlp.plugins.songs.lib.db import Song
324 from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport
325
326@@ -222,7 +221,7 @@
327 # Load the list of songs.
328 self.application.set_busy_cursor()
329 songs = self.plugin.manager.get_all_objects(Song)
330- songs.sort(cmp=natcmp, key=lambda song: song.sort_key)
331+ songs.sort(key=lambda song: song.sort_key)
332 for song in songs:
333 # No need to export temporary songs.
334 if song.temporary:
335
336=== modified file 'openlp/plugins/songs/lib/__init__.py'
337--- openlp/plugins/songs/lib/__init__.py 2013-03-11 21:10:29 +0000
338+++ openlp/plugins/songs/lib/__init__.py 2013-04-06 19:11:20 +0000
339@@ -34,7 +34,7 @@
340 from PyQt4 import QtGui
341
342 from openlp.core.lib import translate
343-from openlp.core.utils import CONTROL_CHARS, locale_direct_compare
344+from openlp.core.utils import CONTROL_CHARS
345 from db import Author
346 from ui import SongStrings
347
348@@ -592,37 +592,3 @@
349 text = u''.join(out)
350 return text, default_encoding
351
352-
353-def natcmp(a, b):
354- """
355- Natural string comparison which mimics the behaviour of Python's internal cmp function.
356- """
357- if len(a) <= len(b):
358- for i, key in enumerate(a):
359- if isinstance(key, int) and isinstance(b[i], int):
360- result = cmp(key, b[i])
361- elif isinstance(key, int) and not isinstance(b[i], int):
362- result = locale_direct_compare(str(key), b[i])
363- elif not isinstance(key, int) and isinstance(b[i], int):
364- result = locale_direct_compare(key, str(b[i]))
365- else:
366- result = locale_direct_compare(key, b[i])
367- if result != 0:
368- return result
369- if len(a) == len(b):
370- return 0
371- else:
372- return -1
373- else:
374- for i, key in enumerate(b):
375- if isinstance(a[i], int) and isinstance(key, int):
376- result = cmp(a[i], key)
377- elif isinstance(a[i], int) and not isinstance(key, int):
378- result = locale_direct_compare(str(a[i]), key)
379- elif not isinstance(a[i], int) and isinstance(key, int):
380- result = locale_direct_compare(a[i], str(key))
381- else:
382- result = locale_direct_compare(a[i], key)
383- if result != 0:
384- return result
385- return 1
386
387=== modified file 'openlp/plugins/songs/lib/db.py'
388--- openlp/plugins/songs/lib/db.py 2013-01-18 23:31:02 +0000
389+++ openlp/plugins/songs/lib/db.py 2013-04-06 19:11:20 +0000
390@@ -38,6 +38,7 @@
391 from sqlalchemy.sql.expression import func
392
393 from openlp.core.lib.db import BaseModel, init_db
394+from openlp.core.utils import get_natural_key
395
396
397 class Author(BaseModel):
398@@ -69,36 +70,15 @@
399 def __init__(self):
400 self.sort_key = ()
401
402- def _try_int(self, s):
403- """
404- Convert to integer if possible.
405- """
406- try:
407- return int(s)
408- except:
409- return s.lower()
410-
411- def _natsort_key(self, s):
412- """
413- Used internally to get a tuple by which s is sorted.
414- """
415- return map(self._try_int, re.findall(r'(\d+|\D+)', s))
416-
417- # This decorator tells sqlalchemy to call this method everytime
418- # any data on this object is updated.
419-
420 @reconstructor
421 def init_on_load(self):
422 """
423- Precompute a tuple to be used for sorting.
424+ Precompute a natural sorting, locale aware sorting key.
425
426 Song sorting is performance sensitive operation.
427- To get maximum speed lets precompute the string
428- used for comparison.
429+ To get maximum speed lets precompute the sorting key.
430 """
431- # Avoid the overhead of converting string to lowercase and to QString
432- # with every call to sort().
433- self.sort_key = self._natsort_key(self.title)
434+ self.sort_key = get_natural_key(self.title)
435
436
437 class Topic(BaseModel):
438
439=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
440--- openlp/plugins/songs/lib/mediaitem.py 2013-03-23 06:46:41 +0000
441+++ openlp/plugins/songs/lib/mediaitem.py 2013-04-06 19:11:20 +0000
442@@ -43,7 +43,7 @@
443 from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm
444 from openlp.plugins.songs.forms.songimportform import SongImportForm
445 from openlp.plugins.songs.forms.songexportform import SongExportForm
446-from openlp.plugins.songs.lib import VerseType, clean_string, natcmp
447+from openlp.plugins.songs.lib import VerseType, clean_string
448 from openlp.plugins.songs.lib.db import Author, Song, Book, MediaFile
449 from openlp.plugins.songs.lib.ui import SongStrings
450 from openlp.plugins.songs.lib.xml import OpenLyrics, SongXML
451@@ -225,7 +225,7 @@
452 log.debug(u'display results Song')
453 self.save_auto_select_id()
454 self.list_view.clear()
455- searchresults.sort(cmp=natcmp, key=lambda song: song.sort_key)
456+ searchresults.sort(key=lambda song: song.sort_key)
457 for song in searchresults:
458 # Do not display temporary songs
459 if song.temporary:
460
461=== modified file 'scripts/check_dependencies.py'
462--- scripts/check_dependencies.py 2013-03-14 10:51:49 +0000
463+++ scripts/check_dependencies.py 2013-04-06 19:11:20 +0000
464@@ -83,6 +83,7 @@
465 'mako',
466 'migrate',
467 'uno',
468+ 'icu',
469 ]
470
471
472
473=== modified file 'tests/functional/openlp_core_utils/test_utils.py'
474--- tests/functional/openlp_core_utils/test_utils.py 2012-12-07 21:38:02 +0000
475+++ tests/functional/openlp_core_utils/test_utils.py 2013-04-06 19:11:20 +0000
476@@ -5,7 +5,7 @@
477
478 from mock import patch
479
480-from openlp.core.utils import get_filesystem_encoding, _get_frozen_path
481+from openlp.core.utils import get_filesystem_encoding, _get_frozen_path, get_locale_key, get_natural_key
482
483 class TestUtils(TestCase):
484 """
485@@ -56,3 +56,30 @@
486 # THEN: The frozen parameter is returned
487 assert _get_frozen_path(u'frozen', u'not frozen') == u'frozen', u'Should return "frozen"'
488
489+ def get_locale_key_test(self):
490+ """
491+ Test the get_locale_key(string) function
492+ """
493+ with patch(u'openlp.core.utils.languagemanager.LanguageManager.get_language') as mocked_get_language:
494+ # GIVEN: The language is German
495+ # 0x00C3 (A with diaresis) should be sorted as "A". 0x00DF (sharp s) should be sorted as "ss".
496+ mocked_get_language.return_value = u'de'
497+ unsorted_list = [u'Auszug', u'Aushang', u'\u00C4u\u00DFerung']
498+ # WHEN: We sort the list and use get_locale_key() to generate the sorting keys
499+ # THEN: We get a properly sorted list
500+ test_passes = sorted(unsorted_list, key=get_locale_key) == [u'Aushang', u'\u00C4u\u00DFerung', u'Auszug']
501+ assert test_passes, u'Strings should be sorted properly'
502+
503+ def get_natural_key_test(self):
504+ """
505+ Test the get_natural_key(string) function
506+ """
507+ with patch(u'openlp.core.utils.languagemanager.LanguageManager.get_language') as mocked_get_language:
508+ # GIVEN: The language is English (a language, which sorts digits before letters)
509+ mocked_get_language.return_value = u'en'
510+ unsorted_list = [u'item 10a', u'item 3b', u'1st item']
511+ # WHEN: We sort the list and use get_natural_key() to generate the sorting keys
512+ # THEN: We get a properly sorted list
513+ test_passes = sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a']
514+ assert test_passes, u'Numbers should be sorted naturally'
515+