Add list plugins method and tests #2496
Conversation
CyclingNinja
force-pushed
the
add_listplugins
branch
4 times, most recently
from
22e7a92
to
c632393
Compare
|
@CyclingNinja - could you try rebasing on upstream/main to get rid of the merge commit? |
| """ | ||
| load_plugins(require_qt_plugins=False) | ||
| plugins = list_plugins() | ||
| assert isinstance(plugins, list) |
There was a problem hiding this comment.
Maybe you can check that certain plugins are in the list
glue/main.py
Outdated
| @@ -115,3 +114,10 @@ def load_plugins(splash=None, require_qt_plugins=False, plugins_to_load=None): | |||
| # that were previously read. | |||
| from glue._settings_helpers import load_settings | |||
| load_settings() | |||
|
|
|||
|
|
|||
| def list_plugins(): | |||
There was a problem hiding this comment.
Thinking about this, I think we might want to actually have list_available_plugins and list_loaded_plugins as there are use cases for both. Could you implement both here?
CyclingNinja
force-pushed
the
add_listplugins
branch
5 times, most recently
from
cc44b8e
to
fe34214
Compare
glue/tests/test_main.py
Outdated
| load_plugins(require_qt_plugins=False) | ||
| plugins = list_plugins() | ||
| assert isinstance(plugins, list) | ||
| assert len(plugins) == 5 |
There was a problem hiding this comment.
Sorry I didn't get back to you sooner about this - rather than checking the absolute number of plugins, you might want to just check that a specific set of them are inside the returned list (but there could be others present)
There was a problem hiding this comment.
The above comment still stands, can you check one of the plugins in the list?
CyclingNinja
force-pushed
the
add_listplugins
branch
4 times, most recently
from
0e397fa
to
6404d9d
Compare
glue/main.py
Outdated
| @@ -3,7 +3,7 @@ | |||
| from importlib import import_module | |||
|
|
|||
| from glue.logger import logger | |||
| from glue._plugin_helpers import REQUIRED_PLUGINS, REQUIRED_PLUGINS_QT | |||
| from glue._plugin_helpers import REQUIRED_PLUGINS | |||
There was a problem hiding this comment.
This change and the one below should be reverted here
| return sorted(_loaded_plugins) | ||
|
|
||
|
|
||
| def list_available_plugins(): |
There was a problem hiding this comment.
Could you import these two functions into the top-level of the package? See:
Line 22 in 5e92185
glue/tests/test_main.py
Outdated
|
|
||
| def test_list_loaded_plugins(): | ||
| """ | ||
| Regression test for retrieving the list of currently loaded plugins |
glue/tests/test_main.py
Outdated
| load_plugins(require_qt_plugins=False) | ||
| plugins = list_plugins() | ||
| assert isinstance(plugins, list) | ||
| assert len(plugins) == 5 |
There was a problem hiding this comment.
The above comment still stands, can you check one of the plugins in the list?
glue/tests/test_main.py
Outdated
| available_plugins = list_available_plugins() | ||
| assert isinstance(available_plugins, list) | ||
| assert len(available_plugins) == 7 | ||
| assert 'casa_formats_io.glue_factory' in available_plugins |
There was a problem hiding this comment.
I'd remove this one, it is only available if glue-astronomy is installed. For checking the absolute number above, maybe first filter it down to just the ones starting with 'glue.' and then you can check how many there are (as otherwise the test will fail if plugin packages are installed)
CyclingNinja
force-pushed
the
add_listplugins
branch
from
55a335e
to
450e6fd
Compare
CyclingNinja
force-pushed
the
add_listplugins
branch
from
450e6fd
to
6169782
Compare
CyclingNinja
force-pushed
the
add_listplugins
branch
from
bd6fd92
to
20787f2
Compare
No description provided.