Repository: regionmask/regionmask

Issue:
natural_earth - ocean_basins_50: mask not wrapping when lon = 0
## Issue 
I think I have an edge case in which case mask data is missing at the dateline when longitude = 0 (in this case the longitude values span 0 : 360). I tried `wrap_lon` = `True`, `False`, `180`, and `360`. Is there a workaround for this? Or do you think there is a possible straightforward fix? 

Aside: Regionmask has been working great – thank you for advancing this library. 

## minimal example
```
import xarray as xr
import regionmask

# open file
fn = "http://aims3.llnl.gov/thredds/dodsC/css03_data/CMIP6/CMIP/CNRM-CERFACS/CNRM-CM6-1/historical/r1i1p1f2/Amon/ts/gr/v20180917/ts_Amon_CNRM-CM6-1_historical_r1i1p1f2_gr_185001-201412.nc" 
ds = xr.open_dataset(fn)

# create mask
ocean = regionmask.defined_regions.natural_earth_v5_0_0.ocean_basins_50
mask = ocean.mask(ds.lon, ds.lat)
mask.plot()
```
![Screen Shot 2023-04-27 at 1 11 45 PM](https://user-images.githubusercontent.com/3698109/234980834-30274f20-e7e8-4507-98e2-e38578ccd9d6.png)


Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/regionmask/regionmask/pull/479?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) Report
> Merging [#479](https://app.codecov.io/gh/regionmask/regionmask/pull/479?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) (e89a09f) into [main](https://app.codecov.io/gh/regionmask/regionmask/commit/7fb7d3bdce7eb3b857a31a11ab7b3ce0d2de4709?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) (7fb7d3b) will **increase** coverage by `0.09%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   98.35%   98.45%   +0.09%     
==========================================
  Files          17       17              
  Lines        1400     1359      -41     
==========================================
- Hits         1377     1338      -39     
+ Misses         23       21       -2     
```

| [Flag](https://app.codecov.io/gh/regionmask/regionmask/pull/479/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | Coverage Δ | |
|---|---|---|
| [bare-minimum](https://app.codecov.io/gh/regionmask/regionmask/pull/479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | `?` | |
| [min-all-deps](https://app.codecov.io/gh/regionmask/regionmask/pull/479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | `?` | |
| [shapely_lt_2](https://app.codecov.io/gh/regionmask/regionmask/pull/479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | `95.06% <100.00%> (+0.35%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/regionmask/regionmask/pull/479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | `98.45% <100.00%> (+0.09%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files](https://app.codecov.io/gh/regionmask/regionmask/pull/479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask) | Coverage Δ | |
|---|---|---|
| [regionmask/defined\_regions/\_\_init\_\_.py](https://app.codecov.io/gh/regionmask/regionmask/pull/479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask#diff-cmVnaW9ubWFzay9kZWZpbmVkX3JlZ2lvbnMvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | |
| [regionmask/defined\_regions/\_natural\_earth.py](https://app.codecov.io/gh/regionmask/regionmask/pull/479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask#diff-cmVnaW9ubWFzay9kZWZpbmVkX3JlZ2lvbnMvX25hdHVyYWxfZWFydGgucHk=) | `98.29% <100.00%> (+1.46%)` | :arrow_up: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/regionmask/regionmask/pull/479/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=regionmask)



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index fbec418..73d47ec 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -17,6 +17,8 @@ Breaking Changes
 ~~~~~~~~~~~~~~~~
 
 - Removed support for Python 3.8 (:pull:`465`).
+- Removed the ``regionmask.defined_regions.natural_earth`` regions which were deprecated
+  in v0.9.0 (:pull:`479`)
 - Removed the deprecated ``subsample`` keyword from the plotting methods (:pull:`468`).
 - Removed the deprecated ``_ar6_pre_revisions`` regions (:pull:`#466`).
 
diff --git a/regionmask/defined_regions/__init__.py b/regionmask/defined_regions/__init__.py
index 32a176d..c526f67 100644
--- a/regionmask/defined_regions/__init__.py
+++ b/regionmask/defined_regions/__init__.py
@@ -2,7 +2,21 @@
 
 from . import _natural_earth, _ressources
 from ._ar6 import ar6
-from ._natural_earth import natural_earth, natural_earth_v4_1_0, natural_earth_v5_0_0
+from ._natural_earth import natural_earth_v4_1_0, natural_earth_v5_0_0
 from .giorgi import giorgi
 from .prudence import prudence
 from .srex import srex
+
+
+def __getattr__(attr_name):
+    # TODO: remove in v0.14.0 or so (added in v0.12.0)
+
+    if attr_name == "natural_earth":
+        raise AttributeError(
+            "The `natural_earth` regions have been removed. Use ``natural_earth_v4_1_0`` "
+            " or ``natural_earth_v5_0_0`` instead."
+        )
+
+    raise AttributeError(
+        f"module 'regionmask.defined_regions' has no attribute '{attr_name}'"
+    )
diff --git a/regionmask/defined_regions/_natural_earth.py b/regionmask/defined_regions/_natural_earth.py
index 93b2751..aa0aa0d 100644
--- a/regionmask/defined_regions/_natural_earth.py
+++ b/regionmask/defined_regions/_natural_earth.py
@@ -1,6 +1,3 @@
-import os
-import warnings
-from abc import ABC, abstractmethod
 from contextlib import contextmanager
 from dataclasses import dataclass
 
@@ -20,13 +17,6 @@ try:
 except ImportError:
     engine = "fiona"
 
-# TODO: remove deprecated (v0.9.0) natural_earth class and instance & clean up
-
-ALTERNATIVE = (
-    "Please use ``regionmask.defined_regions.natural_earth_v4_1_0`` or "
-    "``regionmask.defined_regions.natural_earth_v5_0_0`` instead"
-)
-
 
 def _maybe_get_column(df, colname):
     """return column of the df or not"""
@@ -208,14 +198,16 @@ _ocean_basins_50 = _NaturalEarthFeature(
 )
 
 
-class NaturalEarth(ABC):
+class NaturalEarth:
     """class combining all natural_earth features/ geometries
 
     Because data must be downloaded, we organise it as a class so that
     we only download it on demand.
     """
 
-    def __init__(self):
+    def __init__(self, version, fix_ocean_basins_50):
+        self.version = version
+        self._fix_ocean_basins_50 = fix_ocean_basins_50
 
         self._countries_110 = None
         self._countries_50 = None
@@ -230,9 +222,9 @@ class NaturalEarth(ABC):
 
         self._ocean_basins_50 = None
 
-    @abstractmethod
     def _obtain_ne(self, natural_earth_feature, **kwargs):
-        ...
+        shapefilename = natural_earth_feature.shapefilename(self.version)
+        return _obtain_ne(shapefilename, **kwargs)
 
     @property
     def countries_110(self):
@@ -352,53 +344,28 @@ class NaturalEarth(ABC):
         return self._ocean_basins_50
 
     def __repr__(self):
-        return "Region Definitions from 'http://www.naturalearthdata.com'."
+        return f"Region definitions from 'http://www.naturalearthdata.com' - {self.version}"
 
 
-def _fix_ocean_basins_50_cartopy(self, df):
-    """ocean basins 50 has duplicate entries"""
+def _unify_great_barrier_reef(df, idx1, idx2):
 
-    names_v4_1_0 = {
-        14: "Mediterranean Sea",
-        30: "Mediterranean Sea",
-        26: "Ross Sea",
-        29: "Ross Sea",
-    }
-
-    names_v5_0_0 = {
-        74: "Great Barrier Reef",
-        114: "Great Barrier Reef",
-    }
-
-    names_v5_1_2 = {
-        74: "Great Barrier Reef",
-        113: "Great Barrier Reef",
-    }
-
-    is_v4_1_0 = all(df.loc[idx]["name"] == name for idx, name in names_v4_1_0.items())
-    is_v5_0_0 = all(df.loc[idx]["name"] == name for idx, name in names_v5_0_0.items())
-    is_v5_1_2 = all(df.loc[idx]["name"] == name for idx, name in names_v5_1_2.items())
+    p1 = df.loc[idx1].geometry
+    p2 = df.loc[idx2].geometry
 
-    if is_v4_1_0:
-        df = _fix_ocean_basins_50_v4_1_0(self, df)
-    elif is_v5_0_0:
-        df = _fix_ocean_basins_50_v5_0_0(self, df)
-    elif is_v5_1_2:
-        df = _fix_ocean_basins_50_v5_1_2(self, df)
-    else:
-        raise ValueError(
-            "Unknown version of the ocean basins 50m data from naturalearth. "
-            f"{ALTERNATIVE}."
-        )
+    # merge the two Great Barrier Reef polygons - idx1 <<< idx2
+    poly = p1.union(p2)
+    df.at[idx1, "geometry"] = poly
+    # remove the now merged row
+    df = df.drop(labels=idx2).reset_index()
 
     return df
 
 
-def _fix_ocean_basins_50_v4_1_0(self, df):
+def _fix_ocean_basins_50_v4_1_0(df):
     """fix ocean basins 50 for naturalearth v4.1.0
 
     - Mediterranean Sea and Ross Sea have two parts: renamed to Eastern and Western
-      Basin
+    Basin
     """
 
     new_names = {
@@ -415,21 +382,7 @@ def _fix_ocean_basins_50_v4_1_0(self, df):
     return df
 
 
-def _unify__great_barrier_reef(df, idx1, idx2):
-
-    p1 = df.loc[idx1].geometry
-    p2 = df.loc[idx2].geometry
-
-    # merge the two Great Barrier Reef polygons - idx1 <<< idx2
-    poly = p1.union(p2)
-    df.at[idx1, "geometry"] = poly
-    # remove the now merged row
-    df = df.drop(labels=idx2).reset_index()
-
-    return df
-
-
-def _fix_ocean_basins_50_v5_0_0(self, df):
+def _fix_ocean_basins_50_v5_0_0(df):
     """fix ocean basins 50 for naturalearth v5.0.0
 
     - Mediterranean Sea and Ross Sea is **no longer** split in two.
@@ -437,95 +390,11 @@ def _fix_ocean_basins_50_v5_0_0(self, df):
     - The numbers/ indices are different from Version 4.0!
     """
 
-    return _unify__great_barrier_reef(df, 74, 114)
-
-
-def _fix_ocean_basins_50_v5_1_2(self, df):
-    """fix ocean basins 50 for naturalearth v5.1.2
-
-    - Sea of Japan & Korea Strait geometries are different
-    - the rest (including the split of the Great Barrier Reef) is as in v5.0.0
-    - but the regions are ordered different
-    """
-
-    return _unify__great_barrier_reef(df, 74, 113)
+    return _unify_great_barrier_reef(df, 74, 114)
 
 
-class NaturalEarthCartopy(NaturalEarth):
-
-    _fix_ocean_basins_50 = _fix_ocean_basins_50_cartopy
-
-    def _obtain_ne(self, natural_earth_feature, **kwargs):
-
-        shapefilename = _get_shapefilename_cartopy(
-            natural_earth_feature.resolution,
-            natural_earth_feature.category,
-            natural_earth_feature.name,
-        )
-
-        return _obtain_ne(shapefilename, **kwargs)
-
-
-class NaturalEarth_v4_1_0(NaturalEarth):
-
-    _fix_ocean_basins_50 = _fix_ocean_basins_50_v4_1_0
-    version = "v4.1.0"
-
-    def _obtain_ne(self, natural_earth_feature, **kwargs):
-        shapefilename = natural_earth_feature.shapefilename(self.version)
-        return _obtain_ne(shapefilename, **kwargs)
-
-
-class NaturalEarth_v5_0_0(NaturalEarth):
-
-    _fix_ocean_basins_50 = _fix_ocean_basins_50_v5_0_0
-    version = "v5.0.0"
-
-    def _obtain_ne(self, natural_earth_feature, **kwargs):
-        shapefilename = natural_earth_feature.shapefilename(self.version)
-        return _obtain_ne(shapefilename, **kwargs)
-
-
-natural_earth = NaturalEarthCartopy()
-
-natural_earth_v4_1_0 = NaturalEarth_v4_1_0()
-natural_earth_v5_0_0 = NaturalEarth_v5_0_0()
-
-
-def _get_shapefilename_cartopy(resolution, category, name):
-
-    try:
-        import cartopy
-    except ImportError as e:
-        msg = (
-            "``regionmask.defined_regions.natural_earth`` requires cartopy and is "
-            f"deprecated.\n{ALTERNATIVE} (which do not require cartopy)."
-        )
-        raise ImportError(msg) from e
-
-    _cartopy_data_dir = cartopy.config["data_dir"]
-
-    # check if cartopy has already downloaded the file
-    cartopy_file = os.path.join(
-        _cartopy_data_dir,
-        "shapefiles",
-        "natural_earth",
-        f"{category}",
-        f"ne_{resolution}_{name}.shp",
-    )
-
-    if not os.path.exists(cartopy_file):
-        raise ValueError(
-            "``regionmask.defined_regions.natural_earth`` is deprecated. Will not "
-            f"download new files via this interface. {ALTERNATIVE}."
-        )
-
-    warnings.warn(
-        f"``regionmask.defined_regions.natural_earth`` is deprecated. {ALTERNATIVE}.",
-        FutureWarning,
-    )
-
-    return cartopy_file
+natural_earth_v4_1_0 = NaturalEarth("v4.1.0", _fix_ocean_basins_50_v4_1_0)
+natural_earth_v5_0_0 = NaturalEarth("v5.0.0", _fix_ocean_basins_50_v5_0_0)
 
 
 @contextmanager


Test patch:
diff --git a/regionmask/tests/test_defined_regions.py b/regionmask/tests/test_defined_regions.py
index e82ebb1..22164c6 100644
--- a/regionmask/tests/test_defined_regions.py
+++ b/regionmask/tests/test_defined_regions.py
@@ -1,18 +1,13 @@
-import os
 from operator import attrgetter
 
 import numpy as np
 import pytest
 
 from regionmask import Regions, defined_regions
-from regionmask.defined_regions._natural_earth import _maybe_get_column
+from regionmask.defined_regions._natural_earth import NaturalEarth, _maybe_get_column
 
-from . import has_cartopy, requires_cartopy
-from .utils import (
-    REGIONS_ALL,
-    REGIONS_REQUIRING_CARTOPY,
-    download_naturalearth_region_or_skip,
-)
+from . import requires_cartopy
+from .utils import REGIONS_ALL
 
 
 def _test_region(defined_region):
@@ -37,53 +32,41 @@ def test_defined_region(defined_region):
     _test_region(defined_region)
 
 
-@requires_cartopy
-@pytest.mark.filterwarnings("ignore:Downloading")
-@pytest.mark.parametrize("defined_region", REGIONS_REQUIRING_CARTOPY, ids=str)
-def test_defined_regions_natural_earth(monkeypatch, defined_region):
-    # TODO: remove this test once defined_regions.natural_earth is removed
+def test_defined_regions_natural_earth_informative_error():
 
-    import cartopy
+    with pytest.raises(
+        AttributeError, match="The `natural_earth` regions have been removed."
+    ):
+        defined_regions.natural_earth
 
-    from regionmask.defined_regions import _natural_earth
 
-    match = "``regionmask.defined_regions.natural_earth`` is deprecated"
+def test_defined_regions_attribute_error():
 
-    # get regionmask.defined_regions._natural_earth._land_10 dataclass
-    region = defined_region.region_name.split(".")[1]
-    natural_earth_feature = attrgetter(f"_{region}")(_natural_earth)
+    with pytest.raises(
+        AttributeError,
+        match="module 'regionmask.defined_regions' has no attribute 'attr'",
+    ):
+        defined_regions.attr
 
-    # get the filename of cartopy-downloaded shapefiles
-    resolution = natural_earth_feature.resolution
-    category = natural_earth_feature.category
-    name = natural_earth_feature.name
 
-    _cartopy_data_dir = cartopy.config["data_dir"]
+def test_natural_earth_wrong_version():
+    ne_wrong_version = NaturalEarth("v0.3.0", None)
 
-    # check if cartopy has already downloaded the file
-    cartopy_file = os.path.join(
-        _cartopy_data_dir,
-        "shapefiles",
-        "natural_earth",
-        f"{category}",
-        f"ne_{resolution}_{name}.shp",
-    )
+    with pytest.raises(ValueError, match="version must"):
+        ne_wrong_version.land_110
 
-    # only raise an error if the file is not downloaded
-    if not os.path.isfile(cartopy_file):
-        with pytest.raises(ValueError, match=match):
-            attrgetter(defined_region.region_name)(defined_regions)
 
-    # download data using cartopy
-    download_naturalearth_region_or_skip(monkeypatch, natural_earth_feature)
+def test_natural_earth_repr():
+    actual = repr(defined_regions.natural_earth_v4_1_0)
+    expected = "Region definitions from 'http://www.naturalearthdata.com' - v4.1.0"
+    assert actual == expected
 
-    # get region and ensure deprcation warning is raised
-    with pytest.warns(FutureWarning, match=match):
-        _test_region(defined_region)
+    actual = repr(defined_regions.natural_earth_v5_0_0)
+    expected = "Region definitions from 'http://www.naturalearthdata.com' - v5.0.0"
+    assert actual == expected
 
 
 def test_fix_ocean_basins_50():
-
     region = defined_regions.natural_earth_v4_1_0.ocean_basins_50
     assert "Mediterranean Sea Eastern Basin" in region.names
     assert "Ross Sea Eastern Basin" in region.names
@@ -119,10 +102,3 @@ def test_maybe_get_column():
 
     with pytest.raises(KeyError, match="not on the geopandas dataframe"):
         _maybe_get_column(lowercase, "not_a_column")
-
-
-@pytest.mark.skipif(has_cartopy, reason="should run if cartopy is _not_ installed")
-def test_natural_earth_raises_without_cartopy():
-
-    with pytest.raises(ImportError, match="requires cartopy"):
-        defined_regions.natural_earth.countries_110
diff --git a/regionmask/tests/utils.py b/regionmask/tests/utils.py
index fd065fb..429eaf9 100644
--- a/regionmask/tests/utils.py
+++ b/regionmask/tests/utils.py
@@ -1,10 +1,7 @@
-import warnings
 from dataclasses import dataclass
-from functools import partial
 from typing import Optional
 
 import numpy as np
-import pytest
 import xarray as xr
 
 from regionmask import Regions
@@ -157,36 +154,3 @@ REGIONS += _REGIONS_NATURAL_EARTH_v5_0_0
 
 REGIONS_ALL = REGIONS.copy()
 REGIONS_ALL += _REGIONS_NATURAL_EARTH_v4_1_0
-
-REGIONS_REQUIRING_CARTOPY = [
-    DefinedRegion("natural_earth.countries_110", 177),
-    DefinedRegion("natural_earth.countries_50", 242),
-    DefinedRegion("natural_earth.us_states_50", 51),
-    DefinedRegion("natural_earth.us_states_10", 51),
-    DefinedRegion("natural_earth.land_110", 1),
-    DefinedRegion("natural_earth.land_50", 1),
-    DefinedRegion("natural_earth.land_10", 1),
-    DefinedRegion("natural_earth.ocean_basins_50", 117),
-]
-
-
-def download_naturalearth_region_or_skip(monkeypatch, natural_earth_feature):
-
-    from urllib.request import URLError, urlopen
-
-    import cartopy
-    from cartopy.io import shapereader
-
-    # add a timeout to cartopy.io.urlopen else it can run indefinitely
-    monkeypatch.setattr(cartopy.io, "urlopen", partial(urlopen, timeout=5))
-
-    try:
-        shapereader.natural_earth(
-            natural_earth_feature.resolution,
-            natural_earth_feature.category,
-            natural_earth_feature.name,
-        )
-    except URLError as e:
-        warnings.warn(str(e))
-        warnings.warn("naturalearth download timeout - test not run!")
-        pytest.skip()