Merge pull request #136 from rstudio/fix-compat-r-arrow

fix: support reading type arrow

Author: machow

pins/boards.py

@@ -15,7 +15,7 @@
 from .meta import Meta, MetaRaw, MetaFactory
 from .errors import PinsError
 from .drivers import load_data, save_data, default_title
-from .utils import inform, ExtendMethodDoc
+from .utils import inform, warn_deprecated, ExtendMethodDoc
 from .config import get_allow_rsc_short_name
 
 
@@ -255,6 +255,14 @@ def pin_write(
             part of the pin version name.
         """
 
+        if type == "feather":
+            warn_deprecated(
+                'Writing pin type "feather" is unsupported. Switching type to "arrow".'
+                " This produces the exact same behavior, and also works with R pins."
+                ' Please switch to pin_write using type="arrow".'
+            )
+            type = "arrow"
+
         pin_name = self.path_to_pin(name)
 
         # TODO(docs): describe options for type argument

pins/drivers.py

@@ -76,6 +76,11 @@ def load_data(
 
         return pd.read_csv(fs.open(path_to_file))
 
+    elif meta.type == "arrow":
+        import pandas as pd
+
+        return pd.read_feather(fs.open(path_to_file))
+
     elif meta.type == "feather":
         import pandas as pd
 
@@ -123,11 +128,20 @@ def save_data(
 
         obj.to_csv(final_name, index=False)
 
-    elif type == "feather":
+    elif type == "arrow":
+        # NOTE: R pins accepts the type arrow, and saves it as feather.
+        #       we allow reading this type, but raise an error for writing.
         _assert_is_pandas_df(obj)
 
         obj.to_feather(final_name)
 
+    elif type == "feather":
+        _assert_is_pandas_df(obj)
+
+        raise NotImplementedError(
+            'Saving data as type "feather" no longer supported. Use type "arrow" instead.'
+        )
+
     elif type == "parquet":
         _assert_is_pandas_df(obj)
 

pins/tests/test_boards.py

@@ -96,6 +96,13 @@ class C:
     assert "MY_TYPE" in exc_info.value.args[0]
 
 
+def test_board_pin_write_feather_deprecated(board):
+    df = pd.DataFrame({"x": [1, 2, 3]})
+
+    with pytest.warns(DeprecationWarning):
+        board.pin_write(df, "cool_pin", type="feather")
+
+
 def test_board_pin_write_rsc_index_html(board, tmp_dir2, snapshot):
     if board.fs.protocol != "rsc":
         pytest.skip()

pins/tests/test_drivers.py

@@ -49,8 +49,9 @@ def test_default_title(obj, dst_title):
     "type_",
     [
         "csv",
-        "feather",
+        "arrow",
         "parquet",
+        "joblib",
     ],
 )
 def test_driver_roundtrip(tmp_dir2, type_):
@@ -71,14 +72,41 @@ def test_driver_roundtrip(tmp_dir2, type_):
     assert Path(res_fname).name == full_file
 
     meta = MetaRaw(full_file, type_, "my_pin")
-    obj = load_data(meta, fsspec.filesystem("file"), tmp_dir2)
+    obj = load_data(meta, fsspec.filesystem("file"), tmp_dir2, allow_pickle_read=True)
 
     assert df.equals(obj)
 
 
-@pytest.mark.skip("TODO: complete once driver story is fleshed out")
-def test_driver_roundtrip_joblib(tmp_dir2):
-    pass
+def test_driver_feather_write_error(tmp_dir2):
+    import pandas as pd
+
+    df = pd.DataFrame({"x": [1, 2, 3]})
+
+    fname = "some_df"
+
+    p_obj = tmp_dir2 / fname
+
+    with pytest.raises(NotImplementedError) as exc_info:
+        save_data(df, p_obj, "feather")
+
+    assert '"feather" no longer supported.' in exc_info.value.args[0]
+
+
+def test_driver_feather_read_backwards_compat(tmp_dir2):
+    import pandas as pd
+
+    df = pd.DataFrame({"x": [1, 2, 3]})
+
+    fname = "some_df"
+    full_file = f"{fname}.feather"
+
+    df.to_feather(tmp_dir2 / full_file)
+
+    obj = load_data(
+        MetaRaw(full_file, "feather", "my_pin"), fsspec.filesystem("file"), tmp_dir2
+    )
+
+    assert df.equals(obj)
 
 
 def test_driver_pickle_read_fail_explicit(some_joblib):

pins/utils.py

@@ -2,6 +2,7 @@
 
 from functools import update_wrapper
 from types import MethodType
+from warnings import warn
 
 from .config import pins_options
 
@@ -14,6 +15,10 @@ def inform(log, msg):
         print(msg, file=sys.stderr)
 
 
+def warn_deprecated(msg):
+    warn(msg, DeprecationWarning)
+
+
 class ExtendMethodDoc:
     # Note that the indentation assumes these are top-level method docstrings,
     # so are indented 8 spaces (after the initial sentence).