Remove drop_variables and truthy loadable_variables support

Author: maxrjones

conftest.py

@@ -220,7 +220,7 @@ def netcdf4_file_with_data_in_sibling_groups(tmp_path: Path) -> str:
     filepath = tmp_path / "test.nc"
     ds1 = xr.DataArray([1, 2, 3], name="foo").to_dataset()
     ds1.to_netcdf(filepath, group="subgroup1")
-    ds2 = xr.DataArray([4, 5], name="bar").to_dataset()
+    ds2 = xr.DataArray([4, 5], coords={"x": [0, 1]}, dims="x", name="bar").to_dataset()
     ds2.to_netcdf(filepath, group="subgroup2", mode="a")
     return str(filepath)
 

virtualizarr/manifests/store.py

@@ -336,7 +336,6 @@ def to_virtual_datatree(
         self,
         group="",
         *,
-        drop_variables: Iterable[str] | None = None,
         loadable_variables: Iterable[str] | None = None,
         decode_times: bool | None = None,
     ) -> "xr.DataTree":
@@ -350,8 +349,6 @@ def to_virtual_datatree(
         Parameters
         ----------
         group : Group to convert to a virtual DataTree
-        drop_variables
-            Variables in the data source to drop before returning.
         loadable_variables
             Variables in the data source to load as Dask/NumPy arrays instead of as virtual arrays.
         decode_times
@@ -369,7 +366,6 @@ def to_virtual_datatree(
             group=group,
             loadable_variables=loadable_variables,
             decode_times=decode_times,
-            drop_variables=drop_variables,
         )
 
 

virtualizarr/tests/test_xarray.py

@@ -689,47 +689,60 @@ def test_open_virtual_datatree(
             assert list(vdt["/subgroup1"].variables) == ["foo"]
             assert isinstance(vdt["/subgroup1"]["foo"].data, ManifestArray)
             assert vdt["/subgroup1"]["foo"].shape == (3,)
-            assert list(vdt["/subgroup2"].variables) == ["bar"]
+            assert list(vdt["/subgroup2"].variables) == ["bar", "x"]
             assert isinstance(vdt["/subgroup2"]["bar"].data, ManifestArray)
+            assert isinstance(vdt["/subgroup2"]["x"].data, np.ndarray)
             assert vdt["/subgroup2"]["bar"].shape == (2,)
+            assert vdt["/subgroup2"]["x"].shape == (2,)
 
-    def test_open_virtual_datatree_all_vars_loaded(
+    def test_open_virtual_datatree_no_vars_loaded(
         self, netcdf4_file_with_data_in_sibling_groups, local_registry
     ):
         with (
             open_virtual_datatree(
                 url=netcdf4_file_with_data_in_sibling_groups,
                 registry=local_registry,
                 parser=HDFParser(),
-                loadable_variables=["foo", "bar"],
+                loadable_variables=[],
             ) as vdt,
             open_datatree(
                 netcdf4_file_with_data_in_sibling_groups, engine="h5netcdf"
             ) as dt,
         ):
-            xr.testing.assert_allclose(vdt, dt)
+            vdt.isomorphic(dt)
+            assert list(vdt["/subgroup1"].variables) == ["foo"]
+            assert isinstance(vdt["/subgroup1"]["foo"].data, ManifestArray)
+            assert vdt["/subgroup1"]["foo"].shape == (3,)
+            assert list(vdt["/subgroup2"].variables) == ["bar", "x"]
+            assert isinstance(vdt["/subgroup2"]["bar"].data, ManifestArray)
+            assert isinstance(vdt["/subgroup2"]["x"].data, ManifestArray)
+            assert vdt["/subgroup2"]["bar"].shape == (2,)
+            assert vdt["/subgroup2"]["x"].shape == (2,)
+
+    def test_open_virtual_datatree_all_vars_loaded(
+        self, netcdf4_file_with_data_in_sibling_groups, local_registry
+    ):
+        with pytest.raises(
+            NotImplementedError,
+            match=r"Only `loadable_variables=\[\]` or `loadable_variables=None` are supported, got loadable_variables",
+        ):
+            open_virtual_datatree(
+                url=netcdf4_file_with_data_in_sibling_groups,
+                registry=local_registry,
+                parser=HDFParser(),
+                loadable_variables=["foo", "bar"],
+            )
 
     def test_open_virtual_datatree_drop_vars(
         self, netcdf4_file_with_data_in_sibling_groups, local_registry
     ):
-        with (
+        with pytest.raises(TypeError, match="unexpected keyword argument"):
             open_virtual_datatree(
                 url=netcdf4_file_with_data_in_sibling_groups,
                 registry=local_registry,
                 parser=HDFParser(),
                 drop_variables=["foo"],
-            ) as vdt,
-            open_datatree(
-                netcdf4_file_with_data_in_sibling_groups,
-                engine="h5netcdf",
-                drop_variables=["foo"],
-            ) as dt,
-        ):
-            vdt.isomorphic(dt)
-            assert list(vdt["/subgroup1"].variables) == []
-            assert list(vdt["/subgroup2"].variables) == ["bar"]
-            assert isinstance(vdt["/subgroup2"]["bar"].data, ManifestArray)
-            assert vdt["/subgroup2"]["bar"].shape == (2,)
+            )
 
     @pytest.mark.parametrize("group", ["", None])
     def test_open_root_group(

virtualizarr/xarray.py

@@ -40,7 +40,6 @@ def open_virtual_datatree(
     registry: ObjectStoreRegistry,
     parser: Parser,
     *,
-    drop_variables: Iterable[str] | None = None,
     loadable_variables: Iterable[str] | None = None,
     decode_times: bool | None = None,
 ) -> xr.DataTree:
@@ -72,33 +71,13 @@ def open_virtual_datatree(
         - [virtualizarr.parsers.ZarrParser][] for virtualizing Zarr stores.
         - [virtual_tiff.VirtualTIFF][] for virtualizing TIFFs.
 
-    drop_variables
-        Variables in the data source to drop before returning.
-
-        Variable names are matched by their simple name (not a path). Matching is applied uniformly
-        across all groups in the DataTree. For example, ``drop_variables=["time"]`` will drop any variable
-        named "time" from every group that contains it.
-
-        Path-like strings (e.g., ``"/group1/time"``) are ignored unless they match a specific variable name.
-        To drop variables from specific groups only, open the DataTree first and then use xarray's
-        ``.drop_vars()`` method on the desired nodes.
-
-        Unlike xarray's opening functions, errors are not raised when a specified variable is not found.
-
     loadable_variables
-        Variables in the data source to load as Dask/NumPy arrays instead of as virtual arrays. If
-        ``None`` (the default), dimension coordinate variables (1D variables whose name matches
+        If ``None`` (the default), dimension coordinate variables (1D variables whose name matches
         their dimension) will be loaded automatically to enable xarray indexing.
 
-        Variable names are matched by their simple name (not a path). Matching is applied uniformly
-        across all groups in the DataTree. For example, ``loadable_variables=["time", "lat", "lon"]``
-        will load any variable with those names from every group that contains them.
-
-        Path-like strings (e.g., ``"/group1/time"``) are ignored unless they match a specific variable name.
-        To load variables from specific groups only, you would need to open groups separately using
-        ``open_virtual_dataset`` with the parser's ``group`` parameter.
+        If an empty iterable, no variables will be loaded.
 
-        Unlike xarray's opening functions, errors are not raised when a specified variable is not found.
+        Other options are not yet supported.
 
     decode_times
         Bool that is passed into [xarray.open_dataset][]. Allows time to be decoded into a datetime object.
@@ -162,17 +141,6 @@ def open_virtual_datatree(
     )
     ```
 
-    Drop the "lon" variable from all groups:
-
-    ```python
-    vdt = open_virtual_datatree(
-        url=url,
-        registry=registry,
-        parser=parser,
-        drop_variables=["lon"],
-    )
-    ```
-
     Drop variables from a specific group after opening:
 
     ```python
@@ -187,6 +155,10 @@ def open_virtual_datatree(
     """
     filepath = validate_and_normalize_path_to_uri(url, fs_root=Path.cwd().as_uri())
 
+    if loadable_variables:
+        raise NotImplementedError(
+            f"Only `loadable_variables=[]` or `loadable_variables=None` are supported, got loadable_variables={loadable_variables}"
+        )
     manifest_store = parser(
         url=filepath,
         registry=registry,
@@ -195,7 +167,6 @@ def open_virtual_datatree(
     return manifest_store.to_virtual_datatree(
         loadable_variables=loadable_variables,
         decode_times=decode_times,
-        drop_variables=drop_variables,
     )
 
 
@@ -522,7 +493,6 @@ def construct_virtual_datatree(
     manifest_store: ManifestStore,
     group: str = "",
     *,
-    drop_variables: Iterable[str] | None = None,
     loadable_variables: Iterable[str] | None = None,
     decode_times: bool | None = None,
 ) -> xr.DataTree:
@@ -548,7 +518,7 @@ def construct_virtual_datatree(
             virtual_node.to_dataset(),
             fully_loadable_datatree[name].to_dataset(),
             loadable_variables,
-        ).drop_vars(list(drop_variables or ()), errors="ignore")
+        )
         for name, virtual_node in node.to_virtual_datatree().subtree_with_keys
     }