Repository: pydata/xarray

Issue:
Backend / plugin system `remove_duplicates` raises AttributeError on discovering duplicates
<!-- Please include a self-contained copy-pastable example that generates the issue if possible.

Please be concise with code posted. See guidelines below on how to provide a good bug report:

- Craft Minimal Bug Reports: http://matthewrocklin.com/blog/work/2018/02/28/minimal-bug-reports
- Minimal Complete Verifiable Examples: https://stackoverflow.com/help/mcve

Bug reports that follow these guidelines are easier to diagnose, and so are often handled much more quickly.
-->

**What happened**:

In [one of my CI runs](https://github.com/eX-Mech/pymech/runs/4124742207?check_suite_focus=true#step:7:248) somehow the entrypoints ended up being defined twice. Then, I discovered that the function `remove_duplicates` which weeds out duplicate entrypoints were not updated.

**What you expected to happen**:

No bugs in `remove_duplicates` function

**Minimal Complete Verifiable Example**:

```python
# Put your MCVE code here
from xarray.backends.plugins import remove_duplicates
from importlib.metadata import entry_points

eps = entry_points().get('xarray.backends', ())

remove_duplicates(eps)
remove_duplicates(eps + eps)
```

```
<ipython-input-12-22df5e55614a>:1: DeprecationWarning: EntryPoints list interface is deprecated. Cast to list if needed.
  remove_duplicates(eps + eps)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-12-22df5e55614a> in <module>
----> 1 remove_duplicates(eps + eps)

~/.pyenv/versions/3.9.7/envs/pymech/lib/python3.9/site-packages/xarray/backends/plugins.py in remove_duplicates(entrypoints)
     27         matches_len = len(matches)
     28         if matches_len > 1:
---> 29             selected_module_name = matches[0].module_name
     30             all_module_names = [e.module_name for e in matches]
     31             warnings.warn(

AttributeError: 'EntryPoint' object has no attribute 'module_name'
```

**Anything else we need to know?**:

Following v0.20.0 the entrypoints were discovered using `importlib.metadata` / `importlib_metadata`, but it was broken for 3rd party backeds. In v0.20.1 after #5931 the backend detection was fixed, but remains to be thoroughly tested #5934. This bug might be rare, compared to #5930, so I would recommend having some tests in place before making the next release. 

**Environment**:

<details><summary>Output of <tt>xr.show_versions()</tt></summary>

<!-- Paste the output here xr.show_versions() here -->

INSTALLED VERSIONS
------------------
commit: None
python: 3.9.7 (default, Nov  3 2021, 09:51:04) 
[GCC 11.1.0]
python-bits: 64
OS: Linux
OS-release: 5.10.75-1-lts
machine: x86_64
processor: 
byteorder: little
LC_ALL: None
LANG: C
LOCALE: (None, None)
libhdf5: 1.12.1
libnetcdf: None

xarray: 0.20.1
pandas: 1.3.4
numpy: 1.21.3
scipy: None
netCDF4: None
pydap: None
h5netcdf: 0.11.0
h5py: 3.5.0
Nio: None
zarr: None
cftime: None
nc_time_axis: None
PseudoNetCDF: None
rasterio: 1.2.10
cfgrib: None
iris: None
bottleneck: None
dask: 2021.10.0
distributed: None
matplotlib: None
cartopy: None
seaborn: None
numbagg: None
fsspec: 2021.10.1
cupy: None
pint: None
sparse: None
setuptools: 57.4.0
pip: 21.3.1
conda: None
pytest: 6.2.5
IPython: 7.29.0
sphinx: 4.2.0


</details>


Discussion:
kmuehlbauer: Although this makes the error go away, it doesn't do the right thing. I'll have to think about this more thoroughly.
kmuehlbauer: There seem to be two cases here.

1. equal entrypoints, introduced by importlib machinery (pointing to the same module)
2. multiple entrypoints with the same name (pointing to different modules)

The first is remove via `set`. The second is handled according the old machinery by extracting the modules from `value`.
kmuehlbauer: Finally, some test is breaking. This test uses the old `pkg_resources` way and needs to be updated too.

https://github.com/pydata/xarray/blob/ae3fea0251711c027427578933b82d982a07b242/xarray/tests/test_plugins.py#L29-L45


kmuehlbauer: Obviously we have an extensive set of tests for the backend plugins here (https://github.com/pydata/xarray/blob/ae3fea0251711c027427578933b82d982a07b242/xarray/tests/test_plugins.py) but it still uses `pkg_resources`, so trying to adapt to importlib.

kmuehlbauer: It looks like these changes fix all pending issues. It would be very convenient to get a version `0.20.2` out in near time, since without it breaks testing of packages which implement xarray backends.

@snowman2 I think this is ready for test now with `rioxarray`.
kmuehlbauer: That did not seem to work, I'll revert.
kmuehlbauer: I'll leave it as is for the next couple of days. I very much appreciate further reviews.
kmuehlbauer: It seems that now the mocking works by specifying the correct import module string for `importlib_metadata` depending on which importlib is used.

That should be ready for another review.

Patch:
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index f499cbe3..b66c99d0 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -36,6 +36,8 @@ Bug fixes
 ~~~~~~~~~
 - Fix plot.line crash for data of shape ``(1, N)`` in _title_for_slice on format_item (:pull:`5948`).
   By `Sebastian Weigand <https://github.com/s-weigand>`_.
+- Fix a regression in the removal of duplicate backend entrypoints (:issue:`5944`, :pull:`5959`)
+  By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
 
 Documentation
 ~~~~~~~~~~~~~
@@ -49,6 +51,10 @@ Documentation
 Internal Changes
 ~~~~~~~~~~~~~~~~
 
+- Use ``importlib`` to replace functionality of ``pkg_resources`` in
+  backend plugins tests. (:pull:`5959`).
+  By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
+
 
 .. _whats-new.0.20.1:
 
diff --git a/xarray/backends/plugins.py b/xarray/backends/plugins.py
index a0f46fad..0a9ffcbd 100644
--- a/xarray/backends/plugins.py
+++ b/xarray/backends/plugins.py
@@ -1,15 +1,16 @@
 import functools
 import inspect
 import itertools
+import sys
 import warnings
 
 from .common import BACKEND_ENTRYPOINTS, BackendEntrypoint
 
-try:
+if sys.version_info >= (3, 8):
     from importlib.metadata import entry_points
-except ImportError:
+else:
     # if the fallback library is missing, we are doomed.
-    from importlib_metadata import entry_points  # type: ignore
+    from importlib_metadata import entry_points
 
 
 STANDARD_BACKENDS_ORDER = ["netcdf4", "h5netcdf", "scipy"]
@@ -22,15 +23,17 @@ def remove_duplicates(entrypoints):
     # check if there are multiple entrypoints for the same name
     unique_entrypoints = []
     for name, matches in entrypoints_grouped:
-        matches = list(matches)
+        # remove equal entrypoints
+        matches = list(set(matches))
         unique_entrypoints.append(matches[0])
         matches_len = len(matches)
         if matches_len > 1:
-            selected_module_name = matches[0].module_name
-            all_module_names = [e.module_name for e in matches]
+            all_module_names = [e.value.split(":")[0] for e in matches]
+            selected_module_name = all_module_names[0]
             warnings.warn(
                 f"Found {matches_len} entrypoints for the engine name {name}:"
-                f"\n {all_module_names}.\n It will be used: {selected_module_name}.",
+                f"\n {all_module_names}.\n "
+                f"The entrypoint {selected_module_name} will be used.",
                 RuntimeWarning,
             )
     return unique_entrypoints


Test patch:
diff --git a/xarray/tests/test_plugins.py b/xarray/tests/test_plugins.py
index 7f77a677..4d1eee63 100644
--- a/xarray/tests/test_plugins.py
+++ b/xarray/tests/test_plugins.py
@@ -1,10 +1,20 @@
+import sys
 from unittest import mock
 
-import pkg_resources
 import pytest
 
 from xarray.backends import common, plugins
 
+if sys.version_info >= (3, 8):
+    from importlib.metadata import EntryPoint
+
+    importlib_metadata_mock = "importlib.metadata"
+else:
+    # if the fallback library is missing, we are doomed.
+    from importlib_metadata import EntryPoint
+
+    importlib_metadata_mock = "importlib_metadata"
+
 
 class DummyBackendEntrypointArgs(common.BackendEntrypoint):
     def open_dataset(filename_or_obj, *args):
@@ -29,12 +39,12 @@ class DummyBackendEntrypoint2(common.BackendEntrypoint):
 @pytest.fixture
 def dummy_duplicated_entrypoints():
     specs = [
-        "engine1 = xarray.tests.test_plugins:backend_1",
-        "engine1 = xarray.tests.test_plugins:backend_2",
-        "engine2 = xarray.tests.test_plugins:backend_1",
-        "engine2 = xarray.tests.test_plugins:backend_2",
+        ["engine1", "xarray.tests.test_plugins:backend_1", "xarray.backends"],
+        ["engine1", "xarray.tests.test_plugins:backend_2", "xarray.backends"],
+        ["engine2", "xarray.tests.test_plugins:backend_1", "xarray.backends"],
+        ["engine2", "xarray.tests.test_plugins:backend_2", "xarray.backends"],
     ]
-    eps = [pkg_resources.EntryPoint.parse(spec) for spec in specs]
+    eps = [EntryPoint(name, value, group) for name, value, group in specs]
     return eps
 
 
@@ -46,8 +56,10 @@ def test_remove_duplicates(dummy_duplicated_entrypoints) -> None:
 
 
 def test_broken_plugin() -> None:
-    broken_backend = pkg_resources.EntryPoint.parse(
-        "broken_backend = xarray.tests.test_plugins:backend_1"
+    broken_backend = EntryPoint(
+        "broken_backend",
+        "xarray.tests.test_plugins:backend_1",
+        "xarray.backends",
     )
     with pytest.warns(RuntimeWarning) as record:
         _ = plugins.build_engines([broken_backend])
@@ -68,13 +80,15 @@ def test_remove_duplicates_warnings(dummy_duplicated_entrypoints) -> None:
     assert "entrypoints" in message1
 
 
-@mock.patch("pkg_resources.EntryPoint.load", mock.MagicMock(return_value=None))
+@mock.patch(
+    f"{importlib_metadata_mock}.EntryPoint.load", mock.MagicMock(return_value=None)
+)
 def test_backends_dict_from_pkg() -> None:
     specs = [
-        "engine1 = xarray.tests.test_plugins:backend_1",
-        "engine2 = xarray.tests.test_plugins:backend_2",
+        ["engine1", "xarray.tests.test_plugins:backend_1", "xarray.backends"],
+        ["engine2", "xarray.tests.test_plugins:backend_2", "xarray.backends"],
     ]
-    entrypoints = [pkg_resources.EntryPoint.parse(spec) for spec in specs]
+    entrypoints = [EntryPoint(name, value, group) for name, value, group in specs]
     engines = plugins.backends_dict_from_pkg(entrypoints)
     assert len(engines) == 2
     assert engines.keys() == set(("engine1", "engine2"))
@@ -114,12 +128,12 @@ def test_set_missing_parameters_raise_error() -> None:
 
 
 @mock.patch(
-    "pkg_resources.EntryPoint.load",
+    f"{importlib_metadata_mock}.EntryPoint.load",
     mock.MagicMock(return_value=DummyBackendEntrypoint1),
 )
 def test_build_engines() -> None:
-    dummy_pkg_entrypoint = pkg_resources.EntryPoint.parse(
-        "cfgrib = xarray.tests.test_plugins:backend_1"
+    dummy_pkg_entrypoint = EntryPoint(
+        "cfgrib", "xarray.tests.test_plugins:backend_1", "xarray_backends"
     )
     backend_entrypoints = plugins.build_engines([dummy_pkg_entrypoint])
 
@@ -131,17 +145,13 @@ def test_build_engines() -> None:
 
 
 @mock.patch(
-    "pkg_resources.EntryPoint.load",
+    f"{importlib_metadata_mock}.EntryPoint.load",
     mock.MagicMock(return_value=DummyBackendEntrypoint1),
 )
 def test_build_engines_sorted() -> None:
     dummy_pkg_entrypoints = [
-        pkg_resources.EntryPoint.parse(
-            "dummy2 = xarray.tests.test_plugins:backend_1",
-        ),
-        pkg_resources.EntryPoint.parse(
-            "dummy1 = xarray.tests.test_plugins:backend_1",
-        ),
+        EntryPoint("dummy2", "xarray.tests.test_plugins:backend_1", "xarray.backends"),
+        EntryPoint("dummy1", "xarray.tests.test_plugins:backend_1", "xarray.backends"),
     ]
     backend_entrypoints = plugins.build_engines(dummy_pkg_entrypoints)
     backend_entrypoints = list(backend_entrypoints)