Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds custom list of plugins to be passed on load #2481

Merged
merged 3 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions glue/_plugin_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@
from importlib_metadata import entry_points


REQUIRED_PLUGINS = ['glue.plugins.coordinate_helpers',
'glue.core.data_exporters',
'glue.io.formats.fits']


REQUIRED_PLUGINS_QT = ['glue_qt.plugins.tools.pv_slicer',
'glue_qt.viewers.image',
'glue_qt.viewers.scatter',
'glue_qt.viewers.histogram',
'glue_qt.viewers.profile',
'glue_qt.viewers.table']


def iter_plugin_entry_points():
return iter(entry_points(group='glue.plugins'))

Expand Down
53 changes: 33 additions & 20 deletions glue/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,29 @@
from importlib import import_module

from glue.logger import logger
from glue._plugin_helpers import REQUIRED_PLUGINS, REQUIRED_PLUGINS_QT


_loaded_plugins = set()
_installed_plugins = set()

REQUIRED_PLUGINS = ['glue.plugins.coordinate_helpers',
'glue.core.data_exporters',
'glue.io.formats.fits']

def load_plugins(splash=None, require_qt_plugins=False, plugins_to_use=None):
"""

REQUIRED_PLUGINS_QT = ['glue.plugins.tools.pv_slicer.qt',
'glue.viewers.image.qt',
'glue.viewers.scatter.qt',
'glue.viewers.histogram.qt',
'glue.viewers.profile.qt',
'glue.viewers.table.qt']
Parameters
----------
splash : default: None
instance of splash http rendering service
require_qt_plugins : boolean default: False
whether to use qt plugins defined in constant REQUIRED_PLUGINS_QT
plugins_to_use : list
desired valid plugin strings

Returns
-------

def load_plugins(splash=None, require_qt_plugins=False):
"""

# Search for plugins installed via entry_points. Basically, any package can
# define plugins for glue, and needs to define an entry point using the
Expand All @@ -45,17 +49,26 @@
config = PluginConfig.load()

n_plugins = len(list(iter_plugin_entry_points()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be best to actually define n_plugins as len(plugins_to_use) once plugins_to_use has been defined.

if plugins_to_use is None:
if require_qt_plugins:
plugins_to_use = [*REQUIRED_PLUGINS, *REQUIRED_PLUGINS_QT]

Check warning on line 54 in glue/main.py

View check run for this annotation

Codecov / codecov/patch

glue/main.py#L54

Added line #L54 was not covered by tests
else:
plugins_to_use = REQUIRED_PLUGINS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think plugins_to_use should be based on the REQUIRED_* variables - these are just used to raise exceptions below if they are not present. I think we actually need two different variables inside this function - plugins_to_load(perhaps a better name than plugins_to_use) and plugins_to_require. I think the logic should be:

  • If plugins_to_load is not defined by the user, then plugins_to_load defaults to all entry points (iter_plugin_entry_points) and plugins_to_require defaults to either REQUIRED_PLUGINS or the combination of REQUIRED_PLUGINS and REQUIRED_PLUGINS_QT
  • If plugins_to_load is defined by the user, then plugins_to_require should just be set to plugins_to_load because if the user is explicitly asking for them we should error if they can't be loaded

else:
n_plugins = len(plugins_to_use)

Check warning on line 58 in glue/main.py

View check run for this annotation

Codecov / codecov/patch

glue/main.py#L58

Added line #L58 was not covered by tests

for iplugin, item in enumerate(iter_plugin_entry_points()):
if item.module not in _installed_plugins:
_installed_plugins.add(item.name)
for i_plugin, item in enumerate(list(iter_plugin_entry_points())):
if item.value.replace(':setup', '') in plugins_to_use:
if item.module not in _installed_plugins:
_installed_plugins.add(item.name)

if item.module in _loaded_plugins:
logger.info("Plugin {0} already loaded".format(item.name))
continue
if item.module in _loaded_plugins:
logger.info("Plugin {0} already loaded".format(item.name))
continue

Check warning on line 67 in glue/main.py

View check run for this annotation

Codecov / codecov/patch

glue/main.py#L66-L67

Added lines #L66 - L67 were not covered by tests

if not config.plugins[item.name]:
continue
# loads all plugins, want to make this more customisable
if not config.plugins[item.name]:
continue

Check warning on line 71 in glue/main.py

View check run for this annotation

Codecov / codecov/patch

glue/main.py#L71

Added line #L71 was not covered by tests

# We don't use item.load() because that then checks requirements of all
# the imported packages, which can lead to errors like this one that
Expand All @@ -68,7 +81,7 @@
# old version of a package in the environment, but this can confuse
# users as importing astropy directly would work (as setuptools then
# doesn't do a stringent test of dependency versions). Often this kind
# of error can occur if there is a conda version of a package and and
# of error can occur if there is a conda version of a package and an
# older pip version.

try:
Expand All @@ -90,7 +103,7 @@
_loaded_plugins.add(item.module)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't easily comment on lines above but based on my previous comment,

     if item.module in REQUIRED_PLUGINS:
                raise
            elif item.module in REQUIRED_PLUGINS_QT and require_qt_plugins:
                raise

should then become:

     if item.module in plugins_to_require:
                raise


if splash is not None:
splash.set_progress(100. * iplugin / float(n_plugins))
splash.set_progress(100. * i_plugin / float(n_plugins))

Check warning on line 106 in glue/main.py

View check run for this annotation

Codecov / codecov/patch

glue/main.py#L106

Added line #L106 was not covered by tests

try:
config.save()
Expand Down