From 6aa998edd0f57094aa102912514f50b8a3da5ee0 Mon Sep 17 00:00:00 2001 From: Bastian Germann Date: Sat, 27 Oct 2018 12:50:15 +0200 Subject: [PATCH 1/5] Replace PyICU with PyQt's QCollator Use QCollator as new collator to get rid of the PyICU dependency. Simplify the natural sorting with its numeric mode. Simplify one test that is heavily dependent on implementation. Run one sorting test on macOS which was disabled. --- openlp/core/common/i18n.py | 30 +++++----------- openlp/core/ui/exceptionform.py | 12 ++----- openlp/plugins/songs/lib/mediaitem.py | 6 ++-- scripts/appveyor.yml | 14 ++++---- scripts/check_dependencies.py | 5 ++- setup.py | 9 ++--- .../openlp_core/common/test_i18n.py | 1 - .../openlp_core/ui/test_exceptionform.py | 3 +- .../openlp_plugins/songs/test_mediaitem.py | 34 ++++++++----------- 9 files changed, 41 insertions(+), 73 deletions(-) diff --git a/openlp/core/common/i18n.py b/openlp/core/common/i18n.py index 0f89ed076..7c5b432d1 100644 --- a/openlp/core/common/i18n.py +++ b/openlp/core/common/i18n.py @@ -52,8 +52,7 @@ def translate(context, text, comment=None, qt_translate=QtCore.QCoreApplication. Language = namedtuple('Language', ['id', 'name', 'code']) -ICU_COLLATOR = None -DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+') +COLLATOR = None LANGUAGES = sorted([ Language(1, translate('common.languages', '(Afan) Oromo', 'Language code: om'), 'om'), Language(2, translate('common.languages', 'Abkhazian', 'Language code: ab'), 'ab'), @@ -506,24 +505,19 @@ def format_time(text, local_time): return re.sub(r'\%[a-zA-Z]', match_formatting, text) -def get_locale_key(string): +def get_locale_key(string, numeric=False): """ Creates a key for case insensitive, locale aware string sorting. :param string: The corresponding string. """ string = string.lower() - # ICU is the prefered way to handle locale sort key, we fallback to locale.strxfrm which will work in most cases. - global ICU_COLLATOR - try: - if ICU_COLLATOR is None: - import icu - language = LanguageManager.get_language() - icu_locale = icu.Locale(language) - ICU_COLLATOR = icu.Collator.createInstance(icu_locale) - return ICU_COLLATOR.getSortKey(string) - except Exception: - return locale.strxfrm(string).encode() + global COLLATOR + if COLLATOR is None: + language = LanguageManager.get_language() + COLLATOR = QtCore.QCollator(QtCore.QLocale(language)) + COLLATOR.setNumericMode(numeric) + return COLLATOR.sortKey(string) def get_natural_key(string): @@ -533,13 +527,7 @@ def get_natural_key(string): :param string: string to be sorted by Returns a list of string compare keys and integers. """ - key = DIGITS_OR_NONDIGITS.findall(string) - key = [int(part) if part.isdigit() else get_locale_key(part) for part in key] - # Python 3 does not support comparison of different types anymore. So make sure, that we do not compare str - # and int. - if string and string[0].isdigit(): - return [b''] + key - return key + return get_locale_key(string, True) def get_language(name): diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index 73bcc4b53..4c795c8fb 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -52,14 +52,6 @@ try: MAKO_VERSION = mako.__version__ except ImportError: MAKO_VERSION = '-' -try: - import icu - try: - ICU_VERSION = icu.VERSION - except AttributeError: - ICU_VERSION = 'OK' -except ImportError: - ICU_VERSION = '-' try: WEBKIT_VERSION = QtWebKit.qWebKitVersion() except AttributeError: @@ -119,12 +111,12 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties): system = translate('OpenLP.ExceptionForm', 'Platform: {platform}\n').format(platform=platform.platform()) libraries = ('Python: {python}\nQt5: {qt5}\nPyQt5: {pyqt5}\nQtWebkit: {qtwebkit}\nSQLAlchemy: {sqalchemy}\n' 'SQLAlchemy Migrate: {migrate}\nBeautifulSoup: {soup}\nlxml: {etree}\nChardet: {chardet}\n' - 'PyEnchant: {enchant}\nMako: {mako}\npyICU: {icu}\npyUNO bridge: {uno}\n' + 'PyEnchant: {enchant}\nMako: {mako}\npyUNO bridge: {uno}\n' 'VLC: {vlc}\n').format(python=platform.python_version(), qt5=Qt.qVersion(), pyqt5=Qt.PYQT_VERSION_STR, qtwebkit=WEBKIT_VERSION, sqalchemy=sqlalchemy.__version__, migrate=MIGRATE_VERSION, soup=bs4.__version__, etree=etree.__version__, chardet=CHARDET_VERSION, - enchant=ENCHANT_VERSION, mako=MAKO_VERSION, icu=ICU_VERSION, + enchant=ENCHANT_VERSION, mako=MAKO_VERSION, uno=self._pyuno_import(), vlc=VLC_VERSION) if is_linux(): diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 51f4d0407..ca7f8dffd 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -324,12 +324,12 @@ class SongMediaItem(MediaManagerItem): :param search_results: A tuple containing (songbook entry, book name, song title, song id) :return: None """ - def get_songbook_key(text_array): + def get_songbook_key(text): """ Get the key to sort by - :param text_array: the result text to be processed. + :param text: the text tuple to be processed. """ - return get_natural_key(text_array[1]), get_natural_key(text_array[0]), get_natural_key(text_array[2]) + return get_natural_key('{0} {1} {2}'.format(text[1], text[0], text[2])) log.debug('display results Book') self.list_view.clear() diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index bef8cb5e5..64a771ab3 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -12,19 +12,17 @@ environment: install: # Install dependencies from pypi - "%PYTHON%\\python.exe -m pip install sqlalchemy alembic appdirs chardet beautifulsoup4 lxml Mako mysql-connector-python nose mock pyodbc==4.0.8 psycopg2 pypiwin32==219 pyenchant pymediainfo websockets asyncio waitress six webob requests QtAwesome" - # Download and install pyicu (originally from http://www.lfd.uci.edu/~gohlke/pythonlibs/) - - "%PYTHON%\\python.exe -m pip install https://get.openlp.org/win-sdk/PyICU-1.9.5-cp34-cp34m-win32.whl" # Download and install PyQt5 - - appveyor DownloadFile http://downloads.sourceforge.net/project/pyqt/PyQt5/PyQt-5.5.1/PyQt5-5.5.1-gpl-Py3.4-Qt5.5.1-x32.exe + - appveyor DownloadFile https://downloads.sourceforge.net/project/pyqt/PyQt5/PyQt-5.5.1/PyQt5-5.5.1-gpl-Py3.4-Qt5.5.1-x32.exe - PyQt5-5.5.1-gpl-Py3.4-Qt5.5.1-x32.exe /S # Download and unpack mupdf - - appveyor DownloadFile http://mupdf.com/downloads/archive/mupdf-1.9a-windows.zip - - 7z x mupdf-1.9a-windows.zip - - cp mupdf-1.9a-windows/mupdf.exe openlp-branch/mupdf.exe + - appveyor DownloadFile https://mupdf.com/downloads/archive/mupdf-1.14.0-windows.zip + - 7z x mupdf-1.14.0-windows.zip + - cp mupdf-1.14.0-windows/mupdf.exe openlp-branch/mupdf.exe # Download and unpack mediainfo - - appveyor DownloadFile https://mediaarea.net/download/binary/mediainfo/0.7.90/MediaInfo_CLI_0.7.90_Windows_i386.zip + - appveyor DownloadFile https://mediaarea.net/download/binary/mediainfo/18.08.1/MediaInfo_CLI_18.08.1_Windows_i386.zip - mkdir MediaInfo - - 7z x -oMediaInfo MediaInfo_CLI_0.7.90_Windows_i386.zip + - 7z x -oMediaInfo MediaInfo_CLI_18.08.1_Windows_i386.zip - cp MediaInfo\\MediaInfo.exe openlp-branch\\MediaInfo.exe build: off diff --git a/scripts/check_dependencies.py b/scripts/check_dependencies.py index 3dfb341d8..cffee7bba 100755 --- a/scripts/check_dependencies.py +++ b/scripts/check_dependencies.py @@ -40,8 +40,8 @@ IS_MAC = sys.platform.startswith('dar') VERS = { 'Python': '3.6', - 'PyQt5': '5.0', - 'Qt5': '5.0', + 'PyQt5': '5.5', + 'Qt5': '5.5', 'pymediainfo': '2.2', 'sqlalchemy': '0.5', 'enchant': '1.6' @@ -52,7 +52,6 @@ WIN32_MODULES = [ 'win32com', 'win32ui', 'pywintypes', - 'icu', ] LINUX_MODULES = [ diff --git a/setup.py b/setup.py index bdd4c6e94..4fa594f86 100755 --- a/setup.py +++ b/setup.py @@ -119,7 +119,7 @@ requires = [ 'lxml', 'Mako', 'pymediainfo >= 2.2', - 'PyQt5', + 'PyQt5 >= 5.5', 'QtAwesome', 'requests', 'SQLAlchemy >= 0.5', @@ -128,10 +128,7 @@ requires = [ 'websockets' ] if sys.platform.startswith('win'): - requires.extend([ - 'PyICU', - 'pywin32' - ]) + requires.append('pywin32') elif sys.platform.startswith('darwin'): requires.extend([ 'pyobjc', @@ -204,7 +201,7 @@ using a computer and a data projector.""", 'jenkins': ['python-jenkins'], 'launchpad': ['launchpadlib'] }, - tests_require=['nose2', 'PyICU', 'pylint', 'pyodbc', 'pysword'], + tests_require=['nose2', 'pylint', 'pyodbc', 'pysword'], test_suite='nose2.collector.collector', entry_points={'gui_scripts': ['openlp = run_openlp:start']} ) diff --git a/tests/functional/openlp_core/common/test_i18n.py b/tests/functional/openlp_core/common/test_i18n.py index a4b896c6b..6838bc345 100644 --- a/tests/functional/openlp_core/common/test_i18n.py +++ b/tests/functional/openlp_core/common/test_i18n.py @@ -113,7 +113,6 @@ def test_get_language_invalid_with_none(): assert language is None -@skipIf(is_macosx(), 'This test doesn\'t work on macOS currently') def test_get_locale_key(): """ Test the get_locale_key(string) function diff --git a/tests/functional/openlp_core/ui/test_exceptionform.py b/tests/functional/openlp_core/ui/test_exceptionform.py index 207805f37..71cc419eb 100644 --- a/tests/functional/openlp_core/ui/test_exceptionform.py +++ b/tests/functional/openlp_core/ui/test_exceptionform.py @@ -37,7 +37,6 @@ exceptionform.MIGRATE_VERSION = 'Migrate Test' exceptionform.CHARDET_VERSION = 'CHARDET Test' exceptionform.ENCHANT_VERSION = 'Enchant Test' exceptionform.MAKO_VERSION = 'Mako Test' -exceptionform.ICU_VERSION = 'ICU Test' exceptionform.VLC_VERSION = 'VLC Test' MAIL_ITEM_TEXT = ('**OpenLP Bug Report**\nVersion: Trunk Test\n\n--- Details of the Exception. ---\n\n' @@ -46,7 +45,7 @@ MAIL_ITEM_TEXT = ('**OpenLP Bug Report**\nVersion: Trunk Test\n\n--- Details of 'Python: Python Test\nQt5: Qt5 test\nPyQt5: PyQt5 Test\nQtWebkit: Webkit Test\n' 'SQLAlchemy: SqlAlchemy Test\nSQLAlchemy Migrate: Migrate Test\nBeautifulSoup: BeautifulSoup Test\n' 'lxml: ETree Test\nChardet: CHARDET Test\nPyEnchant: Enchant Test\nMako: Mako Test\n' - 'pyICU: ICU Test\npyUNO bridge: UNO Bridge Test\nVLC: VLC Test\n\n') + 'pyUNO bridge: UNO Bridge Test\nVLC: VLC Test\n\n') @patch("openlp.core.ui.exceptionform.Qt.qVersion") diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index a63e16a72..6b33a1d1d 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -170,27 +170,23 @@ class TestMediaItem(TestCase, TestMixin): """ Test that songbooks are sorted naturally """ - # GIVEN: Search results grouped by book and entry, plus a mocked QtListWidgetItem - with patch('openlp.core.lib.QtWidgets.QListWidgetItem') as MockedQListWidgetItem: - mock_search_results = [('2', 'Thy Book', 'Thy Song', 50), - ('2', 'My Book', 'Your Song', 7), - ('10', 'My Book', 'Our Song', 12), - ('1', 'My Book', 'My Song', 1), - ('2', 'Thy Book', 'A Song', 8)] - mock_qlist_widget = MagicMock() - MockedQListWidgetItem.return_value = mock_qlist_widget + # GIVEN: Search results grouped by book and entry + search_results = [('2', 'Thy Book', 'Thy Song', 50), + ('2', 'My Book', 'Your Song', 7), + ('10', 'My Book', 'Our Song', 12), + ('1', 'My Book', 'My Song', 1), + ('2', 'Thy Book', 'A Song', 8)] - # WHEN: I display song search results grouped by book - self.media_item.display_results_book(mock_search_results) + # WHEN: I display song search results grouped by book + self.media_item.display_results_book(search_results) - # THEN: The songbooks are inserted in the right (natural) order, - # grouped first by book, then by number, then by song title - calls = [call('My Book #1: My Song'), call().setData(QtCore.Qt.UserRole, 1), - call('My Book #2: Your Song'), call().setData(QtCore.Qt.UserRole, 7), - call('My Book #10: Our Song'), call().setData(QtCore.Qt.UserRole, 12), - call('Thy Book #2: A Song'), call().setData(QtCore.Qt.UserRole, 8), - call('Thy Book #2: Thy Song'), call().setData(QtCore.Qt.UserRole, 50)] - MockedQListWidgetItem.assert_has_calls(calls) + # THEN: The songbooks are sorted inplace in the right (natural) order, + # grouped first by book, then by number, then by song title + assert search_results == [('1', 'My Book', 'My Song', 1), + ('2', 'My Book', 'Your Song', 7), + ('10', 'My Book', 'Our Song', 12), + ('2', 'Thy Book', 'A Song', 8), + ('2', 'Thy Book', 'Thy Song', 50)] def test_display_results_topic(self): """ From 191273fbe8b80bb5eac30776608a92f72070eecc Mon Sep 17 00:00:00 2001 From: Bastian Germann Date: Sat, 27 Oct 2018 13:05:41 +0200 Subject: [PATCH 2/5] Remove unused imports --- openlp/core/common/i18n.py | 1 - tests/functional/openlp_core/common/test_i18n.py | 2 -- tests/functional/openlp_plugins/songs/test_mediaitem.py | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/openlp/core/common/i18n.py b/openlp/core/common/i18n.py index 7c5b432d1..b2579d132 100644 --- a/openlp/core/common/i18n.py +++ b/openlp/core/common/i18n.py @@ -23,7 +23,6 @@ The :mod:`languages` module provides a list of language names with utility functions. """ import itertools -import locale import logging import re from collections import namedtuple diff --git a/tests/functional/openlp_core/common/test_i18n.py b/tests/functional/openlp_core/common/test_i18n.py index 6838bc345..2edc44b2e 100644 --- a/tests/functional/openlp_core/common/test_i18n.py +++ b/tests/functional/openlp_core/common/test_i18n.py @@ -22,10 +22,8 @@ """ Package to test the openlp.core.lib.languages package. """ -from unittest import skipIf from unittest.mock import MagicMock, patch -from openlp.core.common import is_macosx from openlp.core.common.i18n import LANGUAGES, Language, UiStrings, get_language, get_locale_key, get_natural_key, \ translate, LanguageManager from openlp.core.common.settings import Settings diff --git a/tests/functional/openlp_plugins/songs/test_mediaitem.py b/tests/functional/openlp_plugins/songs/test_mediaitem.py index 6b33a1d1d..3b0bbe3be 100644 --- a/tests/functional/openlp_plugins/songs/test_mediaitem.py +++ b/tests/functional/openlp_plugins/songs/test_mediaitem.py @@ -23,7 +23,7 @@ This module contains tests for the lib submodule of the Songs plugin. """ from unittest import TestCase -from unittest.mock import MagicMock, patch, call +from unittest.mock import MagicMock, patch from PyQt5 import QtCore From 820476767f23f52966db3500357b262a9d692562 Mon Sep 17 00:00:00 2001 From: Bastian Germann Date: Sat, 27 Oct 2018 23:31:14 +0200 Subject: [PATCH 3/5] Do not export any tests* module --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index bdd4c6e94..c78c3941c 100755 --- a/setup.py +++ b/setup.py @@ -188,7 +188,7 @@ using a computer and a data projector.""", author_email='raoulsnyman@openlp.org', url='https://openlp.org/', license='GNU General Public License', - packages=find_packages(exclude=['ez_setup', 'tests']), + packages=find_packages(exclude=['ez_setup', 'tests*']), py_modules=['run_openlp'], include_package_data=True, zip_safe=False, From 2dc3d42fa9b39f4566b8f974380f0ef508970053 Mon Sep 17 00:00:00 2001 From: Bastian Germann Date: Sun, 28 Oct 2018 12:42:09 +0100 Subject: [PATCH 4/5] Include tests in sdist --- MANIFEST.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MANIFEST.in b/MANIFEST.in index 7bfefe740..9b413cbdd 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -11,7 +11,7 @@ recursive-include openlp *.ttf recursive-include documentation * recursive-include resources * recursive-include scripts * -recursive-include tests/resources * +recursive-include tests * include copyright.txt include LICENSE include README.txt From bad6212008273b1bf028c0558b7bd0b8069a73a8 Mon Sep 17 00:00:00 2001 From: Bastian Germann Date: Mon, 29 Oct 2018 00:24:55 +0100 Subject: [PATCH 5/5] Test Py 3.7 compatibility --- tests/functional/openlp_core/api/http/test_wsgiapp.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_core/api/http/test_wsgiapp.py b/tests/functional/openlp_core/api/http/test_wsgiapp.py index 8f8048b6d..2d087c55c 100644 --- a/tests/functional/openlp_core/api/http/test_wsgiapp.py +++ b/tests/functional/openlp_core/api/http/test_wsgiapp.py @@ -69,7 +69,9 @@ class TestRouting(TestCase): rqst.method = 'GET' application.dispatch(rqst) # THEN: the not found id called - assert 1 == application.route_map['^\\/test\\/image$']['GET'].call_count, \ + route_key = next(iter(application.route_map)) + assert '/image' in route_key + assert 1 == application.route_map[route_key]['GET'].call_count, \ 'main_index function should have been called'