Merge lp:~phill-ridout/openlp/fixes-I into lp:openlp

[Phill]

Fixes a few bugs, and some path lib refactors

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting failed, please see https://ci.openlp.io/job/MP-03-Linting/55/ for more details

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting failed, please see https://ci.openlp.io/job/MP-03-Linting/56/ for more details

[Tim Bentley (trb143)]

Looks ok.

Please do not try and change vlcplayer as you will be on a mission of despair!

review: Approve

[Raoul Snyman (raoul-snyman)]

Linux tests passed!

[Raoul Snyman (raoul-snyman)]

Linting passed!

[Raoul Snyman (raoul-snyman)]

macOS tests failed, please see https://ci.openlp.io/job/MP-04-macOS-Tests/48/ for more details

=== modified file 'openlp/core/api/deploy.py'
--- openlp/core/api/deploy.py	2019-02-14 15:09:09 +0000
+++ openlp/core/api/deploy.py	2019-03-17 21:06:27 +0000
@@ -39,8 +39,8 @@
     :return: None
     """
     zip_path = app_root_path / zip_name
-    web_zip = ZipFile(str(zip_path))
-    web_zip.extractall(str(app_root_path))
+    web_zip = ZipFile(zip_path)
+    web_zip.extractall(app_root_path)
 
 
 def download_sha256():

=== modified file 'openlp/core/app.py'
--- openlp/core/app.py	2019-02-14 15:09:09 +0000
+++ openlp/core/app.py	2019-03-17 21:06:27 +0000
@@ -317,8 +317,7 @@
     """
     create_paths(log_path, do_not_log=True)
     file_path = log_path / 'openlp.log'
-    # TODO: FileHandler accepts a Path object in Py3.6
-    logfile = logging.FileHandler(str(file_path), 'w', encoding='UTF-8')
+    logfile = logging.FileHandler(file_path, 'w', encoding='UTF-8')
     logfile.setFormatter(logging.Formatter('%(asctime)s %(threadName)s %(name)-55s %(levelname)-8s %(message)s'))
     log.addHandler(logfile)
     if log.isEnabledFor(logging.DEBUG):
@@ -364,7 +363,7 @@
         portable_settings_path = data_path / 'OpenLP.ini'
         # Make this our settings file
         log.info('INI file: {name}'.format(name=portable_settings_path))
-        Settings.set_filename(str(portable_settings_path))
+        Settings.set_filename(portable_settings_path)
         portable_settings = Settings()
         # Set our data path
         log.info('Data path: {name}'.format(name=data_path))
@@ -405,7 +404,12 @@
                 None, translate('OpenLP', 'Settings Upgrade'),
                 translate('OpenLP', 'Your settings are about to be upgraded. A backup will be created at '
                                     '{back_up_path}').format(back_up_path=back_up_path))
-            settings.export(back_up_path)
+            try:
+                settings.export(back_up_path)
+            except OSError:
+                QtWidgets.QMessageBox.warning(
+                    None, translate('OpenLP', 'Settings Upgrade'),
+                    translate('OpenLP', 'Settings back up failed.\n\nContinuining to upgrade.'))
         settings.upgrade_settings()
     # First time checks in settings
     if not Settings().value('core/has run wizard'):

=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py	2019-02-14 15:09:09 +0000
+++ openlp/core/common/__init__.py	2019-03-17 21:06:27 +0000
@@ -474,10 +474,10 @@
                 if not chunk:
                     break
                 detector.feed(chunk)
-            detector.close()
-        return detector.result
     except OSError:
         log.exception('Error detecting file encoding')
+    finally:
+        return detector.close()
 
 
 def normalize_str(irregular_string):

=== modified file 'openlp/core/common/path.py'
--- openlp/core/common/path.py	2019-02-14 15:09:09 +0000
+++ openlp/core/common/path.py	2019-03-17 21:06:27 +0000
@@ -78,7 +78,7 @@
         :param onerror: Handler function to handle any errors
         :rtype: None
         """
-        shutil.rmtree(str(self), ignore_errors, onerror)
+        shutil.rmtree(self, ignore_errors, onerror)
 
 
 def replace_params(args, kwargs, params):

=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py	2019-02-14 15:09:09 +0000
+++ openlp/core/common/settings.py	2019-03-17 21:06:27 +0000
@@ -540,7 +540,7 @@
                         old_values = [self._convert_value(old_value, default_value)
                                       for old_value, default_value in zip(old_values, default_values)]
                     # Iterate over our rules and check what the old_value should be "converted" to.
-                    new_value = None
+                    new_value = old_values[0]
                     for new_rule, old_rule in rules:
                         # If the value matches with the condition (rule), then use the provided value. This is used to
                         # convert values. E. g. an old value 1 results in True, and 0 in False.

=== modified file 'openlp/core/display/html/display.html'
--- openlp/core/display/html/display.html	2018-10-12 19:51:51 +0000
+++ openlp/core/display/html/display.html	2019-03-17 21:06:27 +0000
@@ -6,7 +6,7 @@
     <style type="text/css">
     body {
       background: transparent !important;
-      color: #fff !important;
+      color: rgb(255, 255, 255) !important;
     }
     sup {
       vertical-align: super !important;

=== modified file 'openlp/core/display/html/display.js'
--- openlp/core/display/html/display.js	2019-02-05 21:26:30 +0000
+++ openlp/core/display/html/display.js	2019-03-17 21:06:27 +0000
@@ -315,7 +315,7 @@
     section.setAttribute("style", "height: 100%; width: 100%; position: relative;");
     var img = document.createElement('img');
     img.src = image;
-    img.setAttribute("style", "position: absolute; top: 0; bottom: 0; left: 0; right: 0; margin: auto;");
+    img.setAttribute("style", "position: absolute; top: 0; bottom: 0; left: 0; right: 0; margin: auto; max-height: 100%; max-width: 100%");
     section.appendChild(img);
     slidesDiv.appendChild(section);
     Display._slides['0'] = 0;

=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py	2019-02-14 15:09:09 +0000
+++ openlp/core/lib/__init__.py	2019-03-17 21:06:27 +0000
@@ -242,16 +242,16 @@
     """
     Resize an image to fit on the current screen for the web and returns it as a byte stream.
 
-    :param image: The image to converted.
+    :param image: The image to be converted.
     :param base_64: If True returns the image as Base64 bytes, otherwise the image is returned as a byte array.
         To preserve original intention, this defaults to True
     """
     log.debug('image_to_byte - start')
     byte_array = QtCore.QByteArray()
     # use buffer to store pixmap into byteArray
-    buffie = QtCore.QBuffer(byte_array)
-    buffie.open(QtCore.QIODevice.WriteOnly)
-    image.save(buffie, "PNG")
+    buffer = QtCore.QBuffer(byte_array)
+    buffer.open(QtCore.QIODevice.WriteOnly)
+    image.save(buffer, "PNG")
     log.debug('image_to_byte - end')
     if not base_64:
         return byte_array
@@ -263,15 +263,13 @@
     """
     Create a thumbnail from the given image path and depending on ``return_icon`` it returns an icon from this thumb.
 
-    :param image_path: The image file to create the icon from.
-    :param thumb_path: The filename to save the thumbnail to.
+    :param openlp.core.common.path.Path image_path: The image file to create the icon from.
+    :param openlp.core.common.path.Path thumb_path: The filename to save the thumbnail to.
     :param return_icon: States if an icon should be build and returned from the thumb. Defaults to ``True``.
     :param size: Allows to state a own size (QtCore.QSize) to use. Defaults to ``None``, which means that a default
      height of 88 is used.
     :return: The final icon.
     """
-    # TODO: To path object
-    thumb_path = Path(thumb_path)
     reader = QtGui.QImageReader(str(image_path))
     if size is None:
         # No size given; use default height of 88

=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py	2019-02-14 15:09:09 +0000
+++ openlp/core/lib/db.py	2019-03-17 21:06:27 +0000
@@ -132,8 +132,9 @@
     Create a path to a database from the plugin name and database name
 
     :param plugin_name: Name of plugin
-    :param db_file_name: File name of database
-    :return: The path to the database as type str
+    :param openlp.core.common.path.Path | str | None db_file_name: File name of database
+    :return: The path to the database
+    :rtype: str
     """
     if db_file_name is None:
         return 'sqlite:///{path}/{plugin}.sqlite'.format(path=AppLocation.get_section_data_path(plugin_name),
@@ -144,15 +145,15 @@
         return 'sqlite:///{path}/{name}'.format(path=AppLocation.get_section_data_path(plugin_name), name=db_file_name)
 
 
-def handle_db_error(plugin_name, db_file_name):
+def handle_db_error(plugin_name, db_file_path):
     """
     Log and report to the user that a database cannot be loaded
 
     :param plugin_name: Name of plugin
-    :param db_file_name: File name of database
+    :param openlp.core.common.path.Path db_file_path: File name of database
     :return: None
     """
-    db_path = get_db_path(plugin_name, db_file_name)
+    db_path = get_db_path(plugin_name, db_file_path)
     log.exception('Error loading database: {db}'.format(db=db_path))
     critical_error_message_box(translate('OpenLP.Manager', 'Database Error'),
                                translate('OpenLP.Manager',
@@ -161,10 +162,13 @@
 
 def init_url(plugin_name, db_file_name=None):
     """
-    Return the database URL.
+    Construct the connection string for a database.
 
     :param plugin_name: The name of the plugin for the database creation.
-    :param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used.
+    :param openlp.core.common.path.Path | str | None db_file_name: The database file name. Defaults to None resulting
+                                                                   in the plugin_name being used.
+    :return: The database URL
+    :rtype: str
     """
     settings = Settings()
     settings.beginGroup(plugin_name)
@@ -345,7 +349,7 @@
         Runs the initialisation process that includes creating the connection to the database and the tables if they do
         not exist.
 
-        :param plugin_name:  The name to setup paths and settings section names
+        :param plugin_name: The name to setup paths and settings section names
         :param init_schema: The init_schema function for this database
         :param openlp.core.common.path.Path db_file_path: The file name to use for this database. Defaults to None
             resulting in the plugin_name being used.
@@ -357,14 +361,14 @@
         self.db_url = None
         if db_file_path:
             log.debug('Manager: Creating new DB url')
-            self.db_url = init_url(plugin_name, str(db_file_path))
+            self.db_url = init_url(plugin_name, str(db_file_path))  # TOdO :PATHLIB
         else:
             self.db_url = init_url(plugin_name)
         if upgrade_mod:
             try:
                 db_ver, up_ver = upgrade_db(self.db_url, upgrade_mod)
             except (SQLAlchemyError, DBAPIError):
-                handle_db_error(plugin_name, str(db_file_path))
+                handle_db_error(plugin_name, db_file_path)
                 return
             if db_ver > up_ver:
                 critical_error_message_box(
@@ -379,7 +383,7 @@
             try:
                 self.session = init_schema(self.db_url)
             except (SQLAlchemyError, DBAPIError):
-                handle_db_error(plugin_name, str(db_file_path))
+                handle_db_error(plugin_name, db_file_path)
         else:
             self.session = session
 

=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py	2019-02-14 15:09:09 +0000
+++ openlp/core/lib/mediamanageritem.py	2019-03-17 21:06:27 +0000
@@ -454,15 +454,16 @@
         """
         pass
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Live):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Live,
+                            file_path=None):
         """
         Generate the slide data. Needs to be implemented by the plugin.
+
         :param service_item: The service Item to be processed
         :param item: The database item to be used to build the service item
-        :param xml_version:
         :param remote: Was this remote triggered (False)
         :param context: The service context
+        :param openlp.core.common.path.Path file_path:
         """
         raise NotImplementedError('MediaManagerItem.generate_slide_data needs to be defined by the plugin')
 
@@ -624,17 +625,16 @@
                                                             'You must select a {title} '
                                                             'service item.').format(title=self.title))
 
-    def build_service_item(self, item=None, xml_version=False, remote=False, context=ServiceItemContext.Live):
+    def build_service_item(self, item=None, remote=False, context=ServiceItemContext.Live):
         """
         Common method for generating a service item
         :param item: Service Item to be built.
-        :param xml_version: version of XML (False)
         :param remote: Remote triggered (False)
         :param context: The context on which this is called
         """
         service_item = ServiceItem(self.plugin)
         service_item.add_icon()
-        if self.generate_slide_data(service_item, item, xml_version, remote, context):
+        if self.generate_slide_data(service_item, item=item, remote=remote, context=context):
             return service_item
         else:
             return None

=== modified file 'openlp/core/lib/serviceitem.py'
--- openlp/core/lib/serviceitem.py	2019-02-14 15:09:09 +0000
+++ openlp/core/lib/serviceitem.py	2019-03-17 21:06:27 +0000
@@ -263,7 +263,7 @@
             file_location = os.path.join(path, file_name)
             file_location_hash = md5_hash(file_location.encode('utf-8'))
             image = os.path.join(str(AppLocation.get_section_data_path(self.name)), 'thumbnails',
-                                 file_location_hash, ntpath.basename(image))
+                                 file_location_hash, ntpath.basename(image))  # TODO: Pathlib
         self.slides.append({'title': file_name, 'image': image, 'path': path, 'display_title': display_title,
                             'notes': notes, 'thumbnail': image})
         # if self.is_capable(ItemCapabilities.HasThumbnails):
@@ -361,7 +361,7 @@
                 if isinstance(file_path, str):
                     # Handle service files prior to OpenLP 3.0
                     # Windows can handle both forward and backward slashes, so we use ntpath to get the basename
-                    file_path = Path(path, ntpath.basename(file_path))
+                    file_path = path / ntpath.basename(file_path)
                 self.background_audio.append(file_path)
         self.theme_overwritten = header.get('theme_overwritten', False)
         if self.service_item_type == ServiceItemType.Text:
@@ -374,7 +374,7 @@
             if path:
                 self.has_original_files = False
                 for text_image in service_item['serviceitem']['data']:
-                    file_path = os.path.join(path, text_image)
+                    file_path = path / text_image
                     self.add_from_image(file_path, text_image, background)
             else:
                 for text_image in service_item['serviceitem']['data']:

=== modified file 'openlp/core/ui/advancedtab.py'
--- openlp/core/ui/advancedtab.py	2019-03-09 03:53:20 +0000
+++ openlp/core/ui/advancedtab.py	2019-03-17 21:06:27 +0000
@@ -478,7 +478,7 @@
                 minute=self.service_name_time.time().minute()
             )
         try:
-            service_name_example = format_time(str(self.service_name_edit.text()), local_time)
+            service_name_example = format_time(self.service_name_edit.text(), local_time)
         except ValueError:
             preset_is_valid = False
             service_name_example = translate('OpenLP.AdvancedTab', 'Syntax error.')

=== modified file 'openlp/core/ui/exceptiondialog.py'
--- openlp/core/ui/exceptiondialog.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/exceptiondialog.py	2019-03-17 21:06:27 +0000
@@ -77,11 +77,11 @@
         self.save_report_button = create_button(exception_dialog, 'save_report_button',
                                                 icon=UiIcons().save,
                                                 click=self.on_save_report_button_clicked)
-        self.attach_tile_button = create_button(exception_dialog, 'attach_tile_button',
+        self.attach_file_button = create_button(exception_dialog, 'attach_file_button',
                                                 icon=UiIcons().open,
                                                 click=self.on_attach_file_button_clicked)
         self.button_box = create_button_box(exception_dialog, 'button_box', ['close'],
-                                            [self.send_report_button, self.save_report_button, self.attach_tile_button])
+                                            [self.send_report_button, self.save_report_button, self.attach_file_button])
         self.exception_layout.addWidget(self.button_box)
 
         self.retranslate_ui(exception_dialog)
@@ -112,4 +112,4 @@
                       ).format(first_part=exception_part1))
         self.send_report_button.setText(translate('OpenLP.ExceptionDialog', 'Send E-Mail'))
         self.save_report_button.setText(translate('OpenLP.ExceptionDialog', 'Save to File'))
-        self.attach_tile_button.setText(translate('OpenLP.ExceptionDialog', 'Attach File'))
+        self.attach_file_button.setText(translate('OpenLP.ExceptionDialog', 'Attach File'))

=== modified file 'openlp/core/ui/exceptionform.py'
--- openlp/core/ui/exceptionform.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/exceptionform.py	2019-03-17 21:06:27 +0000
@@ -95,12 +95,14 @@
         """
         Saving exception log and system information to a file.
         """
-        file_path, filter_used = FileDialog.getSaveFileName(
-            self,
-            translate('OpenLP.ExceptionForm', 'Save Crash Report'),
-            Settings().value(self.settings_section + '/last directory'),
-            translate('OpenLP.ExceptionForm', 'Text files (*.txt *.log *.text)'))
-        if file_path:
+        while True:
+            file_path, filter_used = FileDialog.getSaveFileName(
+                self,
+                translate('OpenLP.ExceptionForm', 'Save Crash Report'),
+                Settings().value(self.settings_section + '/last directory'),
+                translate('OpenLP.ExceptionForm', 'Text files (*.txt *.log *.text)'))
+            if file_path is None:
+                break
             Settings().setValue(self.settings_section + '/last directory', file_path.parent)
             opts = self._create_report()
             report_text = self.report_text.format(version=opts['version'], description=opts['description'],
@@ -108,8 +110,13 @@
             try:
                 with file_path.open('w') as report_file:
                     report_file.write(report_text)
-            except OSError:
+                    break
+            except OSError as e:
                 log.exception('Failed to write crash report')
+                QtWidgets.QMessageBox.warning(
+                    self, translate('OpenLP.ExceptionDialog', 'Failed to Save Report'),
+                    translate('OpenLP.ExceptionDialog', 'The following error occured when saving the report.\n\n'
+                                                        '{exception}').format(file_name=file_path, exception=e))
 
     def on_send_report_button_clicked(self):
         """

=== modified file 'openlp/core/ui/generaltab.py'
--- openlp/core/ui/generaltab.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/generaltab.py	2019-03-17 21:06:27 +0000
@@ -47,7 +47,6 @@
         """
         Initialise the general settings tab
         """
-        self.logo_file = ':/graphics/openlp-splash-screen.png'
         self.logo_background_color = '#ffffff'
         self.screens = ScreenList()
         self.icon_path = ':/icon/openlp-logo.svg'

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/mainwindow.py	2019-03-17 21:06:27 +0000
@@ -1334,7 +1334,7 @@
                 self.show_status_message(
                     translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} '
                               '- Please wait for copy to finish').format(path=self.new_data_path))
-                dir_util.copy_tree(str(old_data_path), str(self.new_data_path))
+                dir_util.copy_tree(old_data_path, self.new_data_path)
                 self.log_info('Copy successful')
             except (OSError, DistutilsFileError) as why:
                 self.application.set_normal_cursor()

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/servicemanager.py	2019-03-17 21:06:27 +0000
@@ -25,7 +25,6 @@
 import html
 import json
 import os
-import shutil
 import zipfile
 from contextlib import suppress
 from datetime import datetime, timedelta
@@ -234,7 +233,7 @@
         self.service_manager_list.itemExpanded.connect(self.expanded)
         # Last little bits of setting up
         self.service_theme = Settings().value(self.main_window.service_manager_settings_section + '/service theme')
-        self.service_path = str(AppLocation.get_section_data_path('servicemanager'))
+        self.service_path = AppLocation.get_section_data_path('servicemanager')
         # build the drag and drop context menu
         self.dnd_menu = QtWidgets.QMenu()
         self.new_action = self.dnd_menu.addAction(translate('OpenLP.ServiceManager', '&Add New Item'))
@@ -590,11 +589,11 @@
                 self.main_window.increment_progress_bar(service_content_size)
                 # Finally add all the listed media files.
                 for write_path in write_list:
-                    zip_file.write(str(write_path), str(write_path))
+                    zip_file.write(write_path, write_path)
                     self.main_window.increment_progress_bar(write_path.stat().st_size)
                 with suppress(FileNotFoundError):
                     file_path.unlink()
-                os.link(temp_file.name, str(file_path))
+                os.link(temp_file.name, file_path)
             Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent)
         except (PermissionError, OSError) as error:
             self.log_exception('Failed to save service to disk: {name}'.format(name=file_path))
@@ -679,7 +678,7 @@
         service_data = None
         self.application.set_busy_cursor()
         try:
-            with zipfile.ZipFile(str(file_path)) as zip_file:
+            with zipfile.ZipFile(file_path) as zip_file:
                 compressed_size = 0
                 for zip_info in zip_file.infolist():
                     compressed_size += zip_info.compress_size
@@ -692,7 +691,7 @@
                             service_data = json_file.read()
                     else:
                         zip_info.filename = os.path.basename(zip_info.filename)
-                        zip_file.extract(zip_info, str(self.service_path))
+                        zip_file.extract(zip_info, self.service_path)
                     self.main_window.increment_progress_bar(zip_info.compress_size)
             if service_data:
                 items = json.loads(service_data, cls=OpenLPJsonDecoder)
@@ -705,11 +704,13 @@
             else:
                 raise ValidationError(msg='No service data found')
         except (NameError, OSError, ValidationError, zipfile.BadZipFile):
+            self.application.set_normal_cursor()
             self.log_exception('Problem loading service file {name}'.format(name=file_path))
             critical_error_message_box(
                 message=translate('OpenLP.ServiceManager',
-                                  'The service file {file_path} could not be loaded because it is either corrupt, or '
-                                  'not a valid OpenLP 2 or OpenLP 3 service file.'.format(file_path=file_path)))
+                                  'The service file {file_path} could not be loaded because it is either corrupt, '
+                                  'inaccessible, or not a valid OpenLP 2 or OpenLP 3 service file.'
+                                  ).format(file_path=file_path))
         self.main_window.finished_progress_bar()
         self.application.set_normal_cursor()
         self.repaint_service_list(-1, -1)
@@ -1237,11 +1238,11 @@
         """
         Empties the service_path of temporary files on system exit.
         """
-        for file_name in os.listdir(self.service_path):
-            file_path = Path(self.service_path, file_name)
+        for file_path in self.service_path.iterdir():
             delete_file(file_path)
-        if os.path.exists(os.path.join(self.service_path, 'audio')):
-            shutil.rmtree(os.path.join(self.service_path, 'audio'), True)
+        audio_path = self.service_path / 'audio'
+        if audio_path.exists():
+            audio_path.rmtree(True)
 
     def on_theme_combo_box_selected(self, current_index):
         """

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2019-02-14 15:09:09 +0000
+++ openlp/core/ui/thememanager.py	2019-03-17 21:06:27 +0000
@@ -150,7 +150,7 @@
         self.global_theme = Settings().value(self.settings_section + '/global theme')
         self.build_theme_path()
         self.load_first_time_themes()
-        self.upgrade_themes()
+        self.upgrade_themes()  # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed
 
     def bootstrap_post_set_up(self):
         """
@@ -422,10 +422,10 @@
         :rtype: bool
         """
         try:
-            with zipfile.ZipFile(str(theme_path), 'w') as theme_zip:
+            with zipfile.ZipFile(theme_path, 'w') as theme_zip:
                 source_path = self.theme_path / theme_name
                 for file_path in source_path.iterdir():
-                    theme_zip.write(str(file_path), os.path.join(theme_name, file_path.name))
+                    theme_zip.write(file_path, Path(theme_name, file_path.name))
             return True
         except OSError as ose:
             self.log_exception('Export Theme Failed')
@@ -567,10 +567,10 @@
         json_theme = False
         theme_name = ""
         try:
-            with zipfile.ZipFile(str(file_path)) as theme_zip:
+            with zipfile.ZipFile(file_path) as theme_zip:
                 json_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.json']
                 if len(json_file) != 1:
-                    # TODO: remove XML handling after the 2.6 release.
+                    # TODO: remove XML handling after once the upgrade path from 2.4 is no longer required
                     xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
                     if len(xml_file) != 1:
                         self.log_error('Theme contains "{val:d}" theme files'.format(val=len(xml_file)))
@@ -607,12 +607,12 @@
                     else:
                         with full_name.open('wb') as out_file:
                             out_file.write(theme_zip.read(zipped_file))
-        except (OSError, zipfile.BadZipFile):
+        except (OSError, ValidationError, zipfile.BadZipFile):
             self.log_exception('Importing theme from zip failed {name}'.format(name=file_path))
-            raise ValidationError
-        except ValidationError:
-            critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),
-                                       translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
+            critical_error_message_box(
+                translate('OpenLP.ThemeManager', 'Import Error'),
+                translate('OpenLP.ThemeManager', 'There was a problem imoorting {file_name}.\n\nIt is corrupt,'
+                                                 'inaccessible or not a valid theme.').format(file_name=file_path))
         finally:
             if not abort_import:
                 # As all files are closed, we can create the Theme.

=== modified file 'openlp/core/version.py'
--- openlp/core/version.py	2019-02-14 15:09:09 +0000
+++ openlp/core/version.py	2019-03-17 21:06:27 +0000
@@ -200,7 +200,7 @@
     """
     library_versions = OrderedDict([(library, _get_lib_version(*args)) for library, args in LIBRARIES.items()])
     # Just update the dict for display purposes, changing the None to a '-'
-    for library, version in library_versions:
+    for library, version in library_versions.items():
         if version is None:
             library_versions[library] = '-'
     return library_versions

=== modified file 'openlp/core/widgets/edits.py'
--- openlp/core/widgets/edits.py	2019-02-14 15:09:09 +0000
+++ openlp/core/widgets/edits.py	2019-03-17 21:06:27 +0000
@@ -352,7 +352,7 @@
         :rtype: None
         """
         if self._path != path:
-            self._path = path
+            self.path = path
             self.pathChanged.emit(path)
 
 

=== modified file 'openlp/core/widgets/wizard.py'
--- openlp/core/widgets/wizard.py	2019-02-14 15:09:09 +0000
+++ openlp/core/widgets/wizard.py	2019-03-17 21:06:27 +0000
@@ -27,13 +27,11 @@
 from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import is_macosx
-from openlp.core.common.i18n import UiStrings, translate
+from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
-from openlp.core.common.settings import Settings
 from openlp.core.lib.ui import add_welcome_page
 from openlp.core.ui.icons import UiIcons
-from openlp.core.widgets.dialogs import FileDialog
 
 
 log = logging.getLogger(__name__)
@@ -280,41 +278,3 @@
         self.finish_button.setVisible(True)
         self.cancel_button.setVisible(False)
         self.application.process_events()
-
-    def get_file_name(self, title, editbox, setting_name, filters=''):
-        """
-        Opens a FileDialog and saves the filename to the given editbox.
-
-        :param str title: The title of the dialog.
-        :param QtWidgets.QLineEdit editbox:  An QLineEdit.
-        :param str setting_name: The place where to save the last opened directory.
-        :param str filters: The file extension filters. It should contain the file description
-            as well as the file extension. For example::
-
-                'OpenLP 2 Databases (*.sqlite)'
-        :rtype: None
-        """
-        if filters:
-            filters += ';;'
-        filters += '%s (*)' % UiStrings().AllFiles
-        file_path, filter_used = FileDialog.getOpenFileName(
-            self, title, Settings().value(self.plugin.settings_section + '/' + setting_name), filters)
-        if file_path:
-            editbox.setText(str(file_path))
-            Settings().setValue(self.plugin.settings_section + '/' + setting_name, file_path.parent)
-
-    def get_folder(self, title, editbox, setting_name):
-        """
-        Opens a FileDialog and saves the selected folder to the given editbox.
-
-        :param str title: The title of the dialog.
-        :param QtWidgets.QLineEdit editbox: An QLineEditbox.
-        :param str setting_name: The place where to save the last opened directory.
-        :rtype: None
-        """
-        folder_path = FileDialog.getExistingDirectory(
-            self, title, Settings().value(self.plugin.settings_section + '/' + setting_name),
-            FileDialog.ShowDirsOnly)
-        if folder_path:
-            editbox.setText(str(folder_path))
-            Settings().setValue(self.plugin.settings_section + '/' + setting_name, folder_path)

=== modified file 'openlp/plugins/bibles/forms/bibleimportform.py'
--- openlp/plugins/bibles/forms/bibleimportform.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/forms/bibleimportform.py	2019-03-17 21:06:27 +0000
@@ -463,14 +463,14 @@
                         UiStrings().NFSs, translate('BiblesPlugin.ImportWizardForm',
                                                     'You need to specify a file with books of the Bible to use in the '
                                                     'import.'))
-                    self.csv_books_edit.setFocus()
+                    self.csv_books_path_edit.setFocus()
                     return False
                 elif not self.field('csv_versefile'):
                     critical_error_message_box(
                         UiStrings().NFSs,
                         translate('BiblesPlugin.ImportWizardForm', 'You need to specify a file of Bible verses to '
                                                                    'import.'))
-                    self.csv_verses_edit.setFocus()
+                    self.csv_verses_pathedit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.OpenSong:
                 if not self.field('opensong_file'):

=== modified file 'openlp/plugins/bibles/lib/bibleimport.py'
--- openlp/plugins/bibles/lib/bibleimport.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/lib/bibleimport.py	2019-03-17 21:06:27 +0000
@@ -48,9 +48,9 @@
         """
         Check if the supplied file is compressed
 
-        :param file_path: A path to the file to check
+        :param openlp.core.common.path.Path file_path: A path to the file to check
         """
-        if is_zipfile(str(file_path)):
+        if is_zipfile(file_path):
             critical_error_message_box(
                 message=translate('BiblesPlugin.BibleImport',
                                   'The file "{file}" you supplied is compressed. You must decompress it before import.'

=== modified file 'openlp/plugins/bibles/lib/db.py'
--- openlp/plugins/bibles/lib/db.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/lib/db.py	2019-03-17 21:06:27 +0000
@@ -158,11 +158,10 @@
             self.name = kwargs['name']
             if not isinstance(self.name, str):
                 self.name = str(self.name, 'utf-8')
-            # TODO: To path object
-            file_path = Path(clean_filename(self.name) + '.sqlite')
+            self.file_path = Path(clean_filename(self.name) + '.sqlite')
         if 'file' in kwargs:
-            file_path = kwargs['file']
-        Manager.__init__(self, 'bibles', init_schema, file_path, upgrade)
+            self.file_path = kwargs['file']
+        Manager.__init__(self, 'bibles', init_schema, self.file_path, upgrade)
         if self.session and 'file' in kwargs:
             self.get_name()
         self._is_web_bible = None
@@ -750,7 +749,7 @@
         ]
 
 
-class AlternativeBookNamesDB(QtCore.QObject, Manager):
+class AlternativeBookNamesDB(Manager):
     """
     This class represents a database-bound alternative book names system.
     """
@@ -765,8 +764,9 @@
         """
         if AlternativeBookNamesDB.cursor is None:
             file_path = AppLocation.get_directory(AppLocation.DataDir) / 'bibles' / 'alternative_book_names.sqlite'
+            exists = file_path.exists()
             AlternativeBookNamesDB.conn = sqlite3.connect(str(file_path))
-            if not file_path.exists():
+            if not exists:
                 # create new DB, create table alternative_book_names
                 AlternativeBookNamesDB.conn.execute(
                     'CREATE TABLE alternative_book_names(id INTEGER NOT NULL, '

=== modified file 'openlp/plugins/bibles/lib/importers/wordproject.py'
--- openlp/plugins/bibles/lib/importers/wordproject.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/lib/importers/wordproject.py	2019-03-17 21:06:27 +0000
@@ -51,7 +51,7 @@
         Unzip the file to a temporary directory
         """
         self.tmp = TemporaryDirectory()
-        with ZipFile(str(self.file_path)) as zip_file:
+        with ZipFile(self.file_path) as zip_file:
             zip_file.extractall(self.tmp.name)
         self.base_path = Path(self.tmp.name, self.file_path.stem)
 

=== modified file 'openlp/plugins/bibles/lib/manager.py'
--- openlp/plugins/bibles/lib/manager.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/lib/manager.py	2019-03-17 21:06:27 +0000
@@ -187,7 +187,7 @@
         bible = self.db_cache[name]
         bible.session.close_all()
         bible.session = None
-        return delete_file(Path(bible.path, bible.file))
+        return delete_file(bible.path, bible.file_path)
 
     def get_bibles(self):
         """

=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
--- openlp/plugins/bibles/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/bibles/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -911,16 +911,16 @@
             list_widget_items.append(bible_verse)
         return list_widget_items
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         log.debug('generating slide data')
         if item:

=== modified file 'openlp/plugins/custom/lib/mediaitem.py'
--- openlp/plugins/custom/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/custom/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -28,7 +28,7 @@
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
-from openlp.core.lib import ServiceItemContext, check_item_selected
+from openlp.core.lib import check_item_selected
 from openlp.core.lib.mediamanageritem import MediaManagerItem
 from openlp.core.lib.plugin import PluginStatus
 from openlp.core.lib.serviceitem import ItemCapabilities
@@ -219,15 +219,12 @@
         self.search_text_edit.setFocus()
         self.search_text_edit.selectAll()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False,
-                            remote=False, context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
         :param service_item: To be updated
         :param item: The custom database item to be used
-        :param xml_version: No used
-        :param remote: Is this triggered by the Preview Controller or Service Manager.
-        :param context: Why is this item required to be build (Default Service).
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         item_id = self._get_id_of_item_to_generate(item, self.remote_custom)
         service_item.add_capability(ItemCapabilities.CanEdit)

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -542,16 +542,16 @@
         image_items.sort(key=lambda item: get_natural_key(item.text(0)))
         target_group.addChildren(image_items)
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         background = QtGui.QColor(Settings().value(self.settings_section + '/background color'))
         if item:

=== modified file 'openlp/plugins/media/lib/mediaitem.py'
--- openlp/plugins/media/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/media/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -29,7 +29,7 @@
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import UiStrings, get_natural_key, translate
 from openlp.core.common.mixins import RegistryProperties
-from openlp.core.common.path import Path, create_paths, path_to_str
+from openlp.core.common.path import create_paths, path_to_str
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import MediaType, ServiceItemContext, check_item_selected
@@ -166,16 +166,16 @@
         # self.display_type_combo_box.currentIndexChanged.connect(self.override_player_changed)
         pass
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         if item is None:
             item = self.list_view.currentItem()
@@ -229,8 +229,8 @@
         Initialize media item.
         """
         self.list_view.clear()
-        self.service_path = str(AppLocation.get_section_data_path(self.settings_section) / 'thumbnails')
-        create_paths(Path(self.service_path))
+        self.service_path = AppLocation.get_section_data_path(self.settings_section) / 'thumbnails'
+        create_paths(self.service_path)
         self.load_list([path_to_str(file) for file in Settings().value(self.settings_section + '/media files')])
         self.rebuild_players()
 
@@ -264,7 +264,7 @@
         :param media: The media
         :param target_group:
         """
-        media.sort(key=lambda file_name: get_natural_key(os.path.split(str(file_name))[1]))
+        media.sort(key=lambda file_path: get_natural_key(file_path.name))
         for track in media:
             track_info = QtCore.QFileInfo(track)
             item_name = None

=== modified file 'openlp/plugins/presentations/lib/mediaitem.py'
--- openlp/plugins/presentations/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/presentations/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -260,16 +260,16 @@
                     doc.presentation_deleted()
                 doc.close_presentation()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service, file_path=None):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            file_path=None, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         if item:
             items = [item]

=== modified file 'openlp/plugins/presentations/lib/messagelistener.py'
--- openlp/plugins/presentations/lib/messagelistener.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/presentations/lib/messagelistener.py	2019-03-17 21:06:27 +0000
@@ -337,14 +337,8 @@
             # Create a copy of the original item, and then clear the original item so it can be filled with images
             item_cpy = copy.copy(item)
             item.__init__(None)
-            if is_live:
-                # TODO: To Path object
-                self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Live,
-                                                    str(file_path))
-            else:
-                # TODO: To Path object
-                self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Preview,
-                                                    str(file_path))
+            context = ServiceItemContext.Live if is_live else ServiceItemContext.Preview
+            self.media_item.generate_slide_data(item, item=item_cpy, context=context, file_path=file_path)
             # Some of the original serviceitem attributes is needed in the new serviceitem
             item.footer = item_cpy.footer
             item.from_service = item_cpy.from_service

=== modified file 'openlp/plugins/songs/forms/songimportform.py'
--- openlp/plugins/songs/forms/songimportform.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/forms/songimportform.py	2019-03-17 21:06:27 +0000
@@ -329,8 +329,13 @@
             importer = self.plugin.import_songs(
                 source_format,
                 file_paths=self.get_list_of_paths(self.format_widgets[source_format]['file_list_widget']))
-        importer.do_import()
-        self.progress_label.setText(WizardStrings.FinishedImport)
+        try:
+            importer.do_import()
+            self.progress_label.setText(WizardStrings.FinishedImport)
+        except OSError as e:
+            log.exception('Importing songs failed')
+            self.progress_label.setText(translate('SongsPlugin.ImportWizardForm',
+                                                  'Your Song import failed. {error}').format(error=e))
 
     def on_error_copy_to_button_clicked(self):
         """

=== modified file 'openlp/plugins/songs/lib/importers/cclifile.py'
--- openlp/plugins/songs/lib/importers/cclifile.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/lib/importers/cclifile.py	2019-03-17 21:06:27 +0000
@@ -67,7 +67,7 @@
                         details = {'confidence': 1, 'encoding': 'utf-8'}
                     except UnicodeDecodeError:
                         details = chardet.detect(detect_content)
-                in_file = codecs.open(str(file_path), 'r', details['encoding'])
+                in_file = codecs.open(file_path, 'r', details['encoding'])
                 if not in_file.read(1) == '\ufeff':
                     # not UTF or no BOM was found
                     in_file.seek(0)
@@ -251,10 +251,10 @@
         line_number = 0
         check_first_verse_line = False
         verse_text = ''
+        verse_type = VerseType.tags[VerseType.Verse]
         song_author = ''
         verse_start = False
         for line in text_list:
-            verse_type = 'v'
             clean_line = line.strip()
             if not clean_line:
                 if line_number == 0:
@@ -263,6 +263,7 @@
                     if verse_text:
                         self.add_verse(verse_text, verse_type)
                         verse_text = ''
+                        verse_type = VerseType.tags[VerseType.Verse]
                         verse_start = False
             else:
                 # line_number=0, song title
@@ -279,7 +280,7 @@
                     elif not verse_start:
                         # We have the verse descriptor
                         verse_desc_parts = clean_line.split(' ')
-                        if len(verse_desc_parts) == 2:
+                        if len(verse_desc_parts):
                             if verse_desc_parts[0].startswith('Ver'):
                                 verse_type = VerseType.tags[VerseType.Verse]
                             elif verse_desc_parts[0].startswith('Ch'):

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2019-03-17 21:06:27 +0000
@@ -557,16 +557,14 @@
             self.plugin.manager.save_object(new_song)
         self.on_song_list_load()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, context=ServiceItemContext.Service, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
-        :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         log.debug('generate_slide_data: {service}, {item}, {remote}'.format(service=service_item, item=item,
                                                                             remote=self.remote_song))

=== modified file 'openlp/plugins/songusage/forms/songusagedetailform.py'
--- openlp/plugins/songusage/forms/songusagedetailform.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songusage/forms/songusagedetailform.py	2019-03-17 21:06:27 +0000
@@ -85,7 +85,7 @@
             self.main_window.error_message(
                 translate('SongUsagePlugin.SongUsageDetailForm', 'Output Path Not Selected'),
                 translate('SongUsagePlugin.SongUsageDetailForm', 'You have not set a valid output location for your'
-                          ' song usage report. \nPlease select an existing path on your computer.')
+                          ' song usage report.\nPlease select an existing path on your computer.')
             )
             return
         create_paths(path)
@@ -112,7 +112,7 @@
                 self.main_window.information_message(
                     translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation'),
                     translate('SongUsagePlugin.SongUsageDetailForm',
-                              'Report \n{name} \nhas been successfully created. ').format(name=report_file_name)
+                              'Report\n{name}\nhas been successfully created.').format(name=report_file_name)
                 )
         except OSError as ose:
             log.exception('Failed to write out song usage records')

=== modified file 'run_openlp.py'
--- run_openlp.py	2019-02-14 15:09:09 +0000
+++ run_openlp.py	2019-03-17 21:06:27 +0000
@@ -24,6 +24,7 @@
 The entrypoint for OpenLP
 """
 import faulthandler
+import logging
 import multiprocessing
 import sys
 
@@ -34,14 +35,19 @@
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.path import create_paths
 
+log = logging.getLogger(__name__)
+
 
 def set_up_fault_handling():
     """
     Set up the Python fault handler
     """
     # Create the cache directory if it doesn't exist, and enable the fault handler to log to an error log file
-    create_paths(AppLocation.get_directory(AppLocation.CacheDir))
-    faulthandler.enable((AppLocation.get_directory(AppLocation.CacheDir) / 'error.log').open('wb'))
+    try:
+        create_paths(AppLocation.get_directory(AppLocation.CacheDir))
+        faulthandler.enable((AppLocation.get_directory(AppLocation.CacheDir) / 'error.log').open('wb'))
+    except OSError:
+        log.exception('An exception occurred when enabling the fault handler')
 
 
 def start():

=== modified file 'tests/functional/openlp_core/api/test_deploy.py'
--- tests/functional/openlp_core/api/test_deploy.py	2019-02-14 21:19:26 +0000
+++ tests/functional/openlp_core/api/test_deploy.py	2019-03-17 21:06:27 +0000
@@ -63,8 +63,8 @@
         deploy_zipfile(root_path, 'site.zip')
 
         # THEN: the zip file should have been extracted to the right location
-        MockZipFile.assert_called_once_with(root_path_str + os.sep + 'site.zip')
-        mocked_zipfile.extractall.assert_called_once_with(root_path_str)
+        MockZipFile.assert_called_once_with(Path('/tmp/remotes/site.zip'))
+        mocked_zipfile.extractall.assert_called_once_with(Path('/tmp/remotes'))
 
     @patch('openlp.core.api.deploy.Registry')
     @patch('openlp.core.api.deploy.get_web_page')

=== modified file 'tests/functional/openlp_core/common/test_init.py'
--- tests/functional/openlp_core/common/test_init.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_core/common/test_init.py	2019-03-17 21:06:27 +0000
@@ -309,9 +309,9 @@
         """
         # GIVEN: A mocked UniversalDetector instance with done attribute set to True after first iteration
         with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
-                patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open:
+                patch.object(Path, 'open', return_value=BytesIO(b'data' * 260)) as mocked_open:
             encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
-            mocked_universal_detector_inst = MagicMock(result=encoding_result)
+            mocked_universal_detector_inst = MagicMock(**{'close.return_value': encoding_result})
             type(mocked_universal_detector_inst).done = PropertyMock(side_effect=[False, True])
             mocked_universal_detector.return_value = mocked_universal_detector_inst
 
@@ -320,7 +320,7 @@
 
             # THEN: The feed method of UniversalDetector should only br called once before returning a result
             mocked_open.assert_called_once_with('rb')
-            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256)]
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b'data' * 256)]
             mocked_universal_detector_inst.close.assert_called_once_with()
             assert result == encoding_result
 
@@ -331,10 +331,10 @@
         # GIVEN: A mocked UniversalDetector instance which isn't set to done and a mocked open, with 1040 bytes of test
         #       data (enough to run the iterator twice)
         with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
-                patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open:
+                patch.object(Path, 'open', return_value=BytesIO(b'data' * 260)) as mocked_open:
             encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
             mocked_universal_detector_inst = MagicMock(mock=mocked_universal_detector,
-                                                       **{'done': False, 'result': encoding_result})
+                                                       **{'done': False, 'close.return_value': encoding_result})
             mocked_universal_detector.return_value = mocked_universal_detector_inst
 
             # WHEN: Calling get_file_encoding
@@ -342,7 +342,7 @@
 
             # THEN: The feed method of UniversalDetector should have been called twice before returning a result
             mocked_open.assert_called_once_with('rb')
-            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256), call(b"data" * 4)]
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b'data' * 256), call(b'data' * 4)]
             mocked_universal_detector_inst.close.assert_called_once_with()
             assert result == encoding_result
 
@@ -352,13 +352,19 @@
         """
         # GIVEN: A mocked UniversalDetector instance which isn't set to done and a mocked open, with 1040 bytes of test
         #       data (enough to run the iterator twice)
-        with patch('openlp.core.common.UniversalDetector'), \
+        with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
                 patch('builtins.open', side_effect=OSError), \
                 patch('openlp.core.common.log') as mocked_log:
+            encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
+            mocked_universal_detector_inst = MagicMock(mock=mocked_universal_detector,
+                                                       **{'done': False, 'close.return_value': encoding_result})
+            mocked_universal_detector.return_value = mocked_universal_detector_inst
 
             # WHEN: Calling get_file_encoding
             result = get_file_encoding(Path('file name'))
 
             # THEN: log.exception should be called and get_file_encoding should return None
             mocked_log.exception.assert_called_once_with('Error detecting file encoding')
-            assert result is None
+            mocked_universal_detector_inst.feed.assert_not_called()
+            mocked_universal_detector_inst.close.assert_called_once_with()
+            assert result == encoding_result

=== modified file 'tests/functional/openlp_core/common/test_path.py'
--- tests/functional/openlp_core/common/test_path.py	2019-02-14 21:19:26 +0000
+++ tests/functional/openlp_core/common/test_path.py	2019-03-17 21:06:27 +0000
@@ -179,9 +179,8 @@
             # WHEN: Calling :func:`openlp.core.common.path.rmtree` with the path parameter as Path object type
             path.rmtree()
 
-            # THEN: :func:`shutil.rmtree` should have been called with the str equivalents of the Path object.
-            mocked_shutil_rmtree.assert_called_once_with(
-                os.path.join('test', 'path'), False, None)
+            # THEN: :func:`shutil.rmtree` should have been called with the the Path object.
+            mocked_shutil_rmtree.assert_called_once_with(Path('test', 'path'), False, None)
 
     def test_rmtree_optional_params(self):
         """
@@ -198,8 +197,7 @@
 
             # THEN: :func:`shutil.rmtree` should have been called with the optional parameters, with out any of the
             #       values being modified
-            mocked_shutil_rmtree.assert_called_once_with(
-                os.path.join('test', 'path'), True, mocked_on_error)
+            mocked_shutil_rmtree.assert_called_once_with(path, True, mocked_on_error)
 
     def test_which_no_command(self):
         """

=== modified file 'tests/functional/openlp_core/lib/test_serviceitem.py'
--- tests/functional/openlp_core/lib/test_serviceitem.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_core/lib/test_serviceitem.py	2019-03-17 21:06:27 +0000
@@ -141,7 +141,7 @@
         """
         # GIVEN: A new service item and a mocked add icon function
         image_name = 'image_1.jpg'
-        test_file = os.path.join(str(TEST_PATH), image_name)
+        test_file = TEST_PATH / image_name
         frame_array = {'path': test_file, 'title': image_name}
 
         service_item = ServiceItem(None)
@@ -154,13 +154,13 @@
                 mocked_get_section_data_path:
             mocked_exists.return_value = True
             mocked_get_section_data_path.return_value = Path('/path/')
-            service_item.set_from_service(line, str(TEST_PATH))
+            service_item.set_from_service(line, TEST_PATH)
 
         # THEN: We should get back a valid service item
         assert service_item.is_valid is True, 'The new service item should be valid'
         assert test_file == service_item.get_rendered_frame(0), 'The first frame should match the path to the image'
         assert frame_array == service_item.get_frames()[0], 'The return should match frame array1'
-        assert test_file == str(service_item.get_frame_path(0)), \
+        assert test_file == service_item.get_frame_path(0), \
             'The frame path should match the full path to the image'
         assert image_name == service_item.get_frame_title(0), 'The frame title should match the image name'
         assert image_name == service_item.get_display_title(), 'The display title should match the first image name'
@@ -328,7 +328,7 @@
 
         # WHEN: We add a custom from a saved service
         line = convert_file_service_item(TEST_PATH, 'serviceitem-song-linked-audio.osj')
-        service_item.set_from_service(line, '/test/')
+        service_item.set_from_service(line, Path('/test/'))
 
         # THEN: We should get back a valid service item
         assert service_item.is_valid is True, 'The new service item should be valid'

=== modified file 'tests/functional/openlp_core/ui/test_thememanager.py'
--- tests/functional/openlp_core/ui/test_thememanager.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_core/ui/test_thememanager.py	2019-03-17 21:06:27 +0000
@@ -66,9 +66,9 @@
         theme_manager._export_theme(Path('some', 'path', 'Default.otz'), 'Default')
 
         # THEN: The zipfile should be created at the given path
-        mocked_zipfile_init.assert_called_with(os.path.join('some', 'path', 'Default.otz'), 'w')
-        mocked_zipfile_write.assert_called_with(str(RESOURCE_PATH / 'themes' / 'Default' / 'Default.xml'),
-                                                os.path.join('Default', 'Default.xml'))
+        mocked_zipfile_init.assert_called_with(Path('some', 'path', 'Default.otz'), 'w')
+        mocked_zipfile_write.assert_called_with(RESOURCE_PATH / 'themes' / 'Default' / 'Default.xml',
+                                                Path('Default', 'Default.xml'))
 
     def test_initial_theme_manager(self):
         """

=== modified file 'tests/functional/openlp_plugins/bibles/test_manager.py'
--- tests/functional/openlp_plugins/bibles/test_manager.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/bibles/test_manager.py	2019-03-17 21:06:27 +0000
@@ -55,7 +55,8 @@
             instance = BibleManager(MagicMock())
             # We need to keep a reference to the mock for close_all as it gets set to None later on!
             mocked_close_all = MagicMock()
-            mocked_bible = MagicMock(file='KJV.sqlite', path='bibles', **{'session.close_all': mocked_close_all})
+            mocked_bible = MagicMock(file_path='KJV.sqlite', path=Path('bibles'),
+                                     **{'session.close_all': mocked_close_all})
             instance.db_cache = {'KJV': mocked_bible}
 
             # WHEN: Calling delete_bible with 'KJV'
@@ -66,4 +67,4 @@
             assert result is True
             mocked_close_all.assert_called_once_with()
             assert mocked_bible.session is None
-            mocked_delete_file.assert_called_once_with(Path('bibles', 'KJV.sqlite'))
+            mocked_delete_file.assert_called_once_with(Path('bibles'), 'KJV.sqlite')

=== modified file 'tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py'
--- tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py	2019-02-14 15:09:09 +0000
+++ tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py	2019-03-17 21:06:27 +0000
@@ -236,8 +236,8 @@
         assert MockedQListWidgetItem.call_args_list == expected_widget_item_calls, MockedQListWidgetItem.call_args_list
         mocked_author_item1.setData.assert_called_once_with(QtCore.Qt.UserRole, 2)
         mocked_author_item2.setData.assert_called_once_with(QtCore.Qt.UserRole, 1)
-        mocked_authors_list_widget.addItem.call_args_list == [
-            call(mocked_author_item1), call(mocked_author_item2)]
+        mocked_authors_list_widget.addItem.assert_has_calls([
+            call(mocked_author_item1), call(mocked_author_item2)])
 
     @patch('openlp.plugins.songs.forms.songmaintenanceform.QtWidgets.QListWidgetItem')
     @patch('openlp.plugins.songs.forms.songmaintenanceform.Topic')

=== modified file 'tests/openlp_core/common/test_network_interfaces.py'
--- tests/openlp_core/common/test_network_interfaces.py	2019-02-14 15:09:09 +0000
+++ tests/openlp_core/common/test_network_interfaces.py	2019-03-17 21:06:27 +0000
@@ -70,7 +70,7 @@
         """
         Return a QFlags enum with IsUp and IsRunning
         """
-        return (QNetworkInterface.IsUp | QNetworkInterface.IsRunning)
+        return QNetworkInterface.IsUp | QNetworkInterface.IsRunning
 
     def name(self):
         return self.my_name

=== modified file 'tests/openlp_core/projectors/test_projector_pjlink_commands_01.py'
--- tests/openlp_core/projectors/test_projector_pjlink_commands_01.py	2019-03-08 15:19:57 +0000
+++ tests/openlp_core/projectors/test_projector_pjlink_commands_01.py	2019-03-17 21:06:27 +0000
@@ -605,9 +605,9 @@
 
             # THEN: Power should be set to ON
             assert pjlink.power == S_STANDBY, 'Power should not have changed'
-            assert mock_UpdateIcons.emit.called is False, 'projectorUpdateIcons() should not have been called'
-            mock_change_status.called is False, 'change_status() should not have been called'
-            mock_send_command.called is False, 'send_command() should not have been called'
+            mock_UpdateIcons.emit.assert_not_called()
+            mock_change_status.assert_not_called()
+            mock_send_command.assert_not_called()
             mock_log.warning.assert_has_calls(log_warn_calls)
 
     def test_projector_process_powr_off(self):
@@ -627,9 +627,9 @@
 
             # THEN: Power should be set to ON
             assert pjlink.power == S_STANDBY, 'Power should have changed to S_STANDBY'
-            assert mock_UpdateIcons.emit.called is True, 'projectorUpdateIcons should have been called'
-            mock_change_status.called is True, 'change_status should have been called'
-            mock_send_command.called is False, 'send_command should not have been called'
+            mock_UpdateIcons.emit.assert_called_with()
+            mock_change_status.assert_called_with(313)
+            mock_send_command.assert_not_called()
 
     def test_projector_process_rfil_save(self):
         """

=== modified file 'tests/openlp_core/projectors/test_projector_sourceform.py'
--- tests/openlp_core/projectors/test_projector_sourceform.py	2019-02-14 15:09:09 +0000
+++ tests/openlp_core/projectors/test_projector_sourceform.py	2019-03-17 21:06:27 +0000
@@ -83,8 +83,8 @@
         Delete all C++ objects at end so we don't segfault.
         """
         self.projectordb.session.close()
-        del(self.projectordb)
-        del(self.projector)
+        del self.projectordb
+        del self.projector
         retries = 0
         while retries < 5:
             try: