Repository: intake/intake
Pull request #719 (2023-03-07 21:36:41)

Autodiscover 'open_' func when adding to driver registry
Move `make_open_functions()` into DriverSources class as `openers()` method. This will add/remove all the relevant `open_*` functions to intake whenever the registry is updated, not just when intake is first imported.

I needed to rearrange `intake.__getattr__` in order to avoid some circular imports.

Closes https://github.com/intake/intake/issues/717

martindurant: That ended up more of a refactor than I had thought :)
Please give me a little time to check through, but first glance looks OK.
blakerosenthal: > That ended up more of a refactor than I had thought :) Please give me a little time to check through, but first glance looks OK.

Me too. I wanted to `drivers.registered` to reflect the state of all enabled drivers at runtime but realized it only holds plugins registered _at runtime_. Whenever we needed the current state of all the plugins we ran a full scan (entrypoints, config) via `drivers.enabled_plugins()`, but this is also what I needed for the openers. The rearrangement just reflects the cleanup work associated with making `enabled_plugins` a cached instance property rather than an instance method.

Patch:
diff --git a/intake/__init__.py b/intake/__init__.py
index 5b353f9..da4abf9 100644
--- a/intake/__init__.py
+++ b/intake/__init__.py
@@ -31,7 +31,6 @@ imports = {
     "register_driver": "intake.source:register_driver",
     "unregister_driver": "intake.source:unregister_driver",
 }
-openers = set()
 logger = logging.getLogger("intake")
 
 
@@ -45,50 +44,33 @@ def __getattr__(attr):
     must start with "open_", else they will be ignored.
     """
     gl = globals()
-    if attr in openers and attr[:5] == "open_":
-        driver = registry[attr[5:]]  # "open_..."
-        gl[attr] = driver
-    else:
-        if attr in gl:
-            return gl[attr]
-        elif attr in imports:
-            dest = imports[attr]
-            modname = dest.split(":", 1)[0]
-            logger.debug("Importing: %s" % modname)
-            mod = importlib.import_module(modname)
-            if ":" in dest:
-                gl[attr] = getattr(mod, dest.split(":")[1])
-            else:
-                gl[attr] = mod
+
     if attr == "__all__":
         return __dir__()
-    try:
-        return gl[attr]
-    except KeyError:
-        raise AttributeError(attr)
-
 
-def __dir__(*_, **__):
-    return sorted(list(globals()) + list(openers) + list(imports))
+    if attr in gl:
+        return gl[attr]
 
+    if attr in imports:
+        dest = imports[attr]
+        modname = dest.split(":", 1)[0]
+        logger.debug("Importing: %s" % modname)
+        mod = importlib.import_module(modname)
+        if ":" in dest:
+            mod = getattr(mod, dest.split(":")[1])
+        gl[attr] = mod
+        return mod
+
+    if attr[:5] == "open_" and attr[5:] in registry.drivers.enabled_plugins():
+        driver = registry[attr[5:]]  # "open_..."
+        return driver
 
-def make_open_functions():
-    """From the current state of ``registry``, create open_* functions"""
-    from .source.discovery import drivers
+    raise AttributeError(attr)
 
-    for name in drivers.enabled_plugins():
-        func_name = "open_" + name
-        if not func_name.isidentifier():
-            # primitive name normalization
-            func_name = re.sub("[-=~^&|@+]", "_", func_name)
-        if func_name.isidentifier():
-            # stash name for dir() and later fetch
-            openers.add(func_name)
-        else:
-            warnings.warn('Invalid Intake plugin name "%s" found.', name, stacklevel=2)
 
-
-make_open_functions()
+def __dir__(*_, **__):
+    openers = ["open_" + name for name in registry.drivers.enabled_plugins()]
+    return sorted(list(globals()) + list(imports) + openers)
 
 
 def open_catalog(uri=None, **kwargs):
diff --git a/intake/source/discovery.py b/intake/source/discovery.py
index 0532862..4fb632a 100644
--- a/intake/source/discovery.py
+++ b/intake/source/discovery.py
@@ -9,6 +9,7 @@ import importlib
 import inspect
 import logging
 import pkgutil
+import re
 import time
 import warnings
 
@@ -40,7 +41,8 @@ class DriverSouces:
         self.do_scan = do_scan
         self._scanned = None
         self._entrypoints = None
-        self.registered = {}
+        self._registered = None
+        self._disabled = None
 
     @property
     def package_scan(self):
@@ -65,9 +67,6 @@ class DriverSouces:
             if v
         ]
 
-    def disabled(self):
-        return [k for k, v in self.conf.get("drivers", {}).items() if v is False]
-
     def __setitem__(self, key, value):
         super(DriverSouces, self).__setitem__(key, value)
         self.save()
@@ -84,21 +83,31 @@ class DriverSouces:
             self._scanned = _package_scan()
         return self._scanned
 
-    def enabled_plugins(self):
+    def disabled(self):
+        if self._disabled is None:
+            self._disabled = {k for k, v in self.conf.get("drivers", {}).items() if v is False}
+
+        return self._disabled
+
+    def registered(self):
         # priority order (decreasing): runtime, config, entrypoints, package scan
-        out = {}
-        if self.package_scan:
-            out.update(self.scanned)
+        if self._registered is None:
+            out = {}
+            if self.package_scan:
+                out.update(self.scanned)
 
-        for ep in self.from_entrypoints() + self.from_conf():
-            out[ep.name] = ep
+            for ep in self.from_entrypoints() + self.from_conf():
+                out[ep.name] = ep
 
-        out.update(self.registered)
-        out = {k: v for k, v in out.items() if k not in self.disabled()}
-        return out
+            self._registered = out
+
+        return self._registered
+
+    def enabled_plugins(self):
+        return {k: v for k, v in self.registered().items() if k not in self.disabled()}
 
     def register_driver(self, name, value, clobber=False, do_enable=False):
-        """Add runtime driver definition
+        """Add runtime driver definition to list of registered drivers (drivers in global scope with corresponding ``intake.open_*`` function)
 
         Parameters
         ----------
@@ -111,19 +120,21 @@ class DriverSouces:
         do_enable: bool
             If True, unset the disabled flag for this driver
         """
-        if not clobber and name in self.enabled_plugins():
+        name = _normalize(name)
+        if name in self.registered() and not clobber:
             raise ValueError(f"Driver {name} already enabled")
         if name in self.disabled():
             if do_enable:
-                self.enable(name)
+                self.enable(name, value)
             else:
                 logger.warning(f"Adding driver {name}, but it is disabled")
 
-        self.registered[name] = value
+        self.registered()[name] = value
 
     def unregister_driver(self, name):
         """Remove runtime registered driver"""
-        self.registered.pop(name)
+        name = _normalize(name)
+        self.registered().pop(name)
 
     def enable(self, name, driver=None):
         """
@@ -146,6 +157,8 @@ class DriverSouces:
             config["drivers"][name] = driver
         elif config["drivers"].get(name) is False:
             del config["drivers"][name]
+        if name in self.disabled():
+            self.disabled().remove(name)
         config.save()
 
     def disable(self, name):
@@ -158,11 +171,16 @@ class DriverSouces:
         name : string
             As in ``'zarr'``
         """
+        name = _normalize(name)
         config = self.conf
         if "drivers" not in config:
             config["drivers"] = {}
         config["drivers"][name] = False
         config.save()
+        self.disabled().add(name)
+
+
+drivers = DriverSouces()
 
 
 def _load_entrypoint(entrypoint):
@@ -215,7 +233,14 @@ def _package_scan(path=None, plugin_prefix="intake_"):
     return plugins
 
 
-drivers = DriverSouces()
+def _normalize(name):
+    if not name.isidentifier():
+        # primitive name normalization
+        name = re.sub("[-=~^&|@+]", "_", name)
+    if not name.isidentifier():
+        warnings.warn('Invalid Intake plugin name "%s" found.', name, stacklevel=2)
+
+    return name
 
 
 def load_plugins_from_module(module_name):


Test patch:
diff --git a/intake/source/tests/test_discovery.py b/intake/source/tests/test_discovery.py
index e289ba1..e21b7e8 100644
--- a/intake/source/tests/test_discovery.py
+++ b/intake/source/tests/test_discovery.py
@@ -92,8 +92,10 @@ def test_enable_and_disable(extra_pythonpath, tmp_config_path):
 
     try:
         drivers = intake.source.discovery.DriverSouces(do_scan=True)
-        drivers.disable("foo")
         registry = intake.source.DriverRegistry(drivers)
+        assert "foo" in registry
+
+        drivers.disable("foo")
         with pytest.warns(PendingDeprecationWarning):
             assert "foo" in discovery.drivers.scanned
         assert "foo" not in registry
@@ -115,6 +117,23 @@ def test_enable_and_disable(extra_pythonpath, tmp_config_path):
         drivers.enable("some_test_driver", "driver_with_entrypoints.SomeTestDriver")
 
 
+def test_register_and_unregister(extra_pythonpath, tmp_config_path):
+    registry = intake.source.registry
+    assert "bar" not in registry
+    with pytest.raises(ImportError):
+        from intake import open_bar
+
+    intake.register_driver("bar", "intake_foo.FooPlugin")
+    assert "bar" in registry
+    from intake import open_bar  # noqa
+
+    intake.unregister_driver("bar")
+    assert "bar" not in registry
+
+    with pytest.raises(ImportError):
+        from intake import open_bar  # noqa
+
+
 def test_discover_collision(extra_pythonpath, tmp_config_path):
     with pytest.warns(UserWarning):
         discovery._package_scan(plugin_prefix="collision_")