← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~phill-ridout/openlp/pathlib3 into lp:openlp

 

See inline


Diff comments:

> === modified file 'openlp/core/__init__.py'
> --- openlp/core/__init__.py	2017-08-01 20:59:41 +0000
> +++ openlp/core/__init__.py	2017-08-12 19:11:29 +0000
> @@ -391,7 +395,7 @@
>          Settings.setDefaultFormat(Settings.IniFormat)
>          # Get location OpenLPPortable.ini
>          application_path = str(AppLocation.get_directory(AppLocation.AppDir))
> -        set_up_logging(os.path.abspath(os.path.join(application_path, '..', '..', 'Other')))
> +        set_up_logging(Path(os.path.abspath(os.path.join(application_path, '..', '..', 'Other'))))

Until Py3.6, the Path equivalent Path.resolve (https://docs.python.org/3/library/pathlib.html#pathlib.Path.resolve) is a concrete method and only works on existing paths (as it resolves symlinks), if the path does not exist it raises FileNotFoundError. From Py3.6 a strict argument was added, which if set to False, allows it to calculate any non existent part of the Path.

This is one of the reasons I wanted to use the back ported pathlib2. However, I have reworked this code in a later revision and eliminated the use of os.path.

>          log.info('Running portable')
>          portable_settings_file = os.path.abspath(os.path.join(application_path, '..', '..', 'Data', 'OpenLP.ini'))
>          # Make this our settings file
> 
> === modified file 'openlp/core/lib/pluginmanager.py'
> --- openlp/core/lib/pluginmanager.py	2017-08-01 20:59:41 +0000
> +++ openlp/core/lib/pluginmanager.py	2017-08-12 19:11:29 +0000
> @@ -69,7 +69,7 @@
>          """
>          Scan a directory for objects inheriting from the ``Plugin`` class.
>          """
> -        glob_pattern = os.path.join('openlp', 'plugins', '*', '*plugin.py')
> +        glob_pattern = os.path.join('plugins', '*', '*plugin.py')

Its 6 of one and half a dozen of another. To me, this makes more sense. We're only going to be searching for modules to import within the OpenLP application directory, rather than the parent folder (see lines 108 and 111)  By adding a very slight complexity to to line 138, we can simplify lines 111, 390, 436, 869.

The documentation for path_to_module, could be improved and make mention that the path should be a sub path of the application directory.

>          extension_loader(glob_pattern)
>          plugin_classes = Plugin.__subclasses__()
>          plugin_objects = []
> 
> === modified file 'openlp/core/ui/media/mediacontroller.py'
> --- openlp/core/ui/media/mediacontroller.py	2017-05-30 18:50:39 +0000
> +++ openlp/core/ui/media/mediacontroller.py	2017-08-12 19:11:29 +0000
> @@ -174,7 +174,7 @@
>          Check to see if we have any media Player's available.
>          """
>          log.debug('_check_available_media_players')
> -        controller_dir = os.path.join('openlp', 'core', 'ui', 'media')
> +        controller_dir = os.path.join('core', 'ui', 'media')
>          glob_pattern = os.path.join(controller_dir, '*player.py')

See above

>          extension_loader(glob_pattern, ['mediaplayer.py'])
>          player_classes = MediaPlayer.__subclasses__()


-- 
https://code.launchpad.net/~phill-ridout/openlp/pathlib3/+merge/328950
Your team OpenLP Core is subscribed to branch lp:openlp.


References