support unversioned boards (#230)

* support unversioned boards

* rsconnect boards cannot delete active version

* clean up function sig

* add versioned arg

* updates from review

* add tests

* try to make smart-ish API calls for Connect

* use test value

* don't need to test the error twice

* also, don't need to read pin

Author: isabelizimm

pins/boards.py

@@ -11,9 +11,9 @@
 
 from typing import Sequence, Optional, Mapping
 
-from .versions import VersionRaw, guess_version
+from .versions import VersionRaw, guess_version, version_setup
 from .meta import Meta, MetaRaw, MetaFactory
-from .errors import PinsError
+from .errors import PinsError, PinsVersionError
 from .drivers import load_data, save_data, load_file, default_title
 from .utils import inform, warn_deprecated, ExtendMethodDoc
 from .config import get_allow_rsc_short_name
@@ -90,16 +90,14 @@ def pin_versions(self, name: str, as_df: bool = True) -> Sequence[VersionRaw]:
             Pin name.
 
         """
-        if not self.versioned:
-            raise NotImplementedError(
-                "Cannot show versions for a board type that does not support versioning."
-            )
 
         if not self.pin_exists(name):
             raise PinsError("Cannot check version, since pin %s does not exist" % name)
 
+        detail = isinstance(self, BoardRsConnect)
+
         versions_raw = self.fs.ls(
-            self.construct_path([self.path_to_pin(name)]), detail=False
+            self.construct_path([self.path_to_pin(name)]), detail=detail
         )
 
         # get a list of Version(Raw) objects
@@ -111,7 +109,6 @@ def pin_versions(self, name: str, as_df: bool = True) -> Sequence[VersionRaw]:
         # sort them, with latest last
         sorted_versions = self.sort_pin_versions(all_versions)
 
-        # TODO(defer): this deviates from R pins, which returns a df by default
         if as_df:
             import pandas as pd
 
@@ -241,11 +238,6 @@ def _pin_store(
         created: Optional[datetime] = None,
     ) -> Meta:
 
-        if not self.versioned:
-            raise NotImplementedError(
-                "Can only write pins with boards that support versioning."
-            )
-
         if type == "feather":
             warn_deprecated(
                 'Writing pin type "feather" is unsupported. Switching type to "arrow".'
@@ -360,11 +352,6 @@ def pin_write(
             part of the pin version name.
         """
 
-        if not self.versioned:
-            raise NotImplementedError(
-                "Can only write pins with boards that support versioning."
-            )
-
         if type == "file":
             raise NotImplementedError(
                 ".pin_write() does not support type='file'. "
@@ -463,10 +450,6 @@ def pin_version_delete(self, name: str, version: str):
         version:
             Version identifier.
         """
-        if not self.versioned:
-            raise NotImplementedError(
-                "Can only write pins with boards that support versioning."
-            )
 
         pin_name = self.path_to_pin(name)
 
@@ -616,7 +599,7 @@ def pin_browse(self, name, version=None, local=False):
     def validate_pin_name(self, name: str) -> None:
         """Raise an error if a pin name is not valid."""
 
-        if "/" in name:
+        if name and "/" in name:
             raise ValueError(f"Invalid pin name: {name}")
         elif name in self.reserved_pin_names:
             raise ValueError(f"The pin name '{name}' is reserved for internal use.")
@@ -654,13 +637,41 @@ def prepare_pin_version(
         versioned: Optional[bool] = None,
         created: Optional[datetime] = None,
         object_name: Optional[str] = None,
+    ):
+        meta = self._create_meta(
+            pin_dir_path,
+            x,
+            name,
+            type,
+            title,
+            description,
+            metadata,
+            versioned,
+            created,
+            object_name,
+        )
+
+        # handle unversioned boards
+        version_setup(self, name, meta.version, versioned)
+
+        return meta
+
+    def _create_meta(
+        self,
+        pin_dir_path,
+        x,
+        name: Optional[str] = None,
+        type: Optional[str] = None,
+        title: Optional[str] = None,
+        description: Optional[str] = None,
+        metadata: Optional[Mapping] = None,
+        versioned: Optional[bool] = None,
+        created: Optional[datetime] = None,
+        object_name: Optional[str] = None,
     ):
         if name is None:
             raise NotImplementedError("Name must be specified.")
 
-        if versioned is False:
-            raise NotImplementedError("Only writing versioned pins supported.")
-
         if type is None:
             raise NotImplementedError("Type argument is required.")
 
@@ -770,15 +781,6 @@ def __init__(self, *args, pin_paths: dict, **kwargs):
 
         self.pin_paths = pin_paths
 
-    # def pin_read(self, *args, **kwargs):
-    #    if not get_feature_preview():
-    #        raise NotImplementedError(
-    #            "pin_read with BoardManual is currently unimplemented. "
-    #            "See https://github.com/machow/pins-python/issues/59."
-    #        )
-
-    #    return super().pin_read(*args, **kwargs)
-
     @ExtendMethodDoc
     def pin_list(self):
         return list(self.pin_paths)
@@ -889,7 +891,9 @@ def pin_list(self):
         return names
 
     @ExtendMethodDoc
-    def pin_write(self, *args, access_type=None, **kwargs):
+    def pin_write(
+        self, *args, access_type=None, versioned: Optional[bool] = None, **kwargs
+    ):
         """Write a pin.
 
         Extends parent method in the following ways:
@@ -915,6 +919,28 @@ def pin_write(self, *args, access_type=None, **kwargs):
                 " can write to it."
             )
 
+        # attempt to make the least number of API calls possible
+        if versioned or versioned is None and self.versioned:
+            # arbitrary number greater than 1
+            n_versions_before = 100
+        else:
+            try:
+                versions_df = self.pin_versions(pin_name, as_df=True)
+                versions = versions_df["version"].to_list()
+                n_versions_before = len(versions)
+            except PinsError:
+                # pin does not exist
+                n_versions_before = 0
+
+        if versioned is None:
+            versioned = True if n_versions_before > 1 else self.versioned
+
+        if versioned is False and n_versions_before > 1:
+            raise PinsVersionError(
+                "Pin is versioned, but you have requested a write without versions."
+                "To un-version a pin, you must delete it"
+            )
+
         # run parent function ---
         meta = f_super(*args, **kwargs)
 
@@ -928,6 +954,12 @@ def pin_write(self, *args, access_type=None, **kwargs):
             access_type=access_type or content["access_type"],
         )
 
+        # clean up non-active pins in the case of an unversioned board
+        # a pin existed before the latest pin
+        if versioned is False and n_versions_before == 1:
+            _log.info(f"Replacing version '{versions}' with '{meta.version.version}'")
+            self.pin_version_delete(pin_name, versions[0])
+
         return meta
 
     @ExtendMethodDoc
@@ -1067,7 +1099,7 @@ def prepare_pin_version(self, pin_dir_path, x, name: "str | None", *args, **kwar
 
         # TODO(compat): py pins always uses the short name, R pins uses w/e the
         # user passed, but guessing people want the long name?
-        meta = super().prepare_pin_version(pin_dir_path, x, short_name, *args, **kwargs)
+        meta = super()._create_meta(pin_dir_path, x, short_name, *args, **kwargs)
         meta.name = name
 
         # copy in files needed by index.html ----------------------------------

pins/tests/helpers.py

@@ -114,7 +114,7 @@ def get_param_from_env(entry):
 
         return value
 
-    def create_tmp_board(self, src_board=None) -> BaseBoard:
+    def create_tmp_board(self, src_board=None, versioned=True) -> BaseBoard:
         if self.fs_name == "gcs":
             opts = {"cache_timeout": 0}
         else:
@@ -136,7 +136,7 @@ def create_tmp_board(self, src_board=None) -> BaseBoard:
             fs.mkdir(board_name)
 
         self.board_path_registry.append(board_name)
-        return BaseBoard(board_name, fs=fs)
+        return BaseBoard(board_name, fs=fs, versioned=versioned)
 
     def teardown_board(self, board):
         board.fs.rm(board.board, recursive=True)
@@ -168,10 +168,10 @@ def __init__(self, fs_name, path=None, *args, **kwargs):
         self.fs_name = fs_name
         self.path = None
 
-    def create_tmp_board(self, src_board=None):
+    def create_tmp_board(self, src_board=None, versioned=True):
         from pins.rsconnect.fs import PinBundleManifest  # noqa
 
-        board = BoardRsConnect("", rsc_fs_from_key("derek"))
+        board = BoardRsConnect("", rsc_fs_from_key("derek"), versioned=versioned)
 
         if src_board is None:
             return board

pins/tests/test_boards.py

@@ -6,7 +6,7 @@
 
 from pins.tests.helpers import DEFAULT_CREATION_DATE, rm_env
 from pins.config import PINS_ENV_INSECURE_READ
-from pins.errors import PinsError, PinsInsecureReadError
+from pins.errors import PinsError, PinsInsecureReadError, PinsVersionError
 from pins.meta import MetaRaw
 
 from datetime import datetime, timedelta
@@ -31,6 +31,12 @@ def board(backend):
     backend.teardown()
 
 
+@fixture
+def board_unversioned(backend):
+    yield backend.create_tmp_board(versioned=False)
+    backend.teardown()
+
+
 @fixture
 def board_with_cache(backend):
     from pins.constructors import board as board_constructor, board_rsconnect
@@ -304,6 +310,36 @@ def test_board_pin_read_insecure_succeed_board_flag(board):
         board.pin_read("test_pin")
 
 
+# pin_write with unversioned boards ===========================================
+
+
+@pytest.mark.parametrize("versioned", [None, False])
+def test_board_unversioned_pin_write_unversioned(versioned, board_unversioned):
+    board_unversioned.pin_write({"a": 1}, "test_pin", type="json", versioned=versioned)
+    board_unversioned.pin_write({"a": 2}, "test_pin", type="json", versioned=versioned)
+
+    assert len(board_unversioned.pin_versions("test_pin")) == 1
+    assert board_unversioned.pin_read("test_pin") == {"a": 2}
+
+
+def test_board_unversioned_pin_write_versioned(board_unversioned):
+    board_unversioned.pin_write({"a": 1}, "test_pin", type="json", versioned=False)
+    board_unversioned.pin_write({"a": 2}, "test_pin", type="json", versioned=True)
+
+    assert len(board_unversioned.pin_versions("test_pin")) == 2
+
+
+def test_board_versioned_pin_write_unversioned(board):
+    # should fall back to the versioned setting of the board
+    board.pin_write({"a": 1}, "test_pin", type="json")
+    board.pin_write({"a": 2}, "test_pin", type="json")
+
+    with pytest.raises(PinsVersionError):
+        board.pin_write({"a": 3}, "test_pin", type="json", versioned=False)
+
+    assert len(board.pin_versions("test_pin")) == 2
+
+
 # pin_delete ==================================================================
 
 

pins/versions.py

@@ -1,12 +1,15 @@
+import logging
+
 from dataclasses import dataclass, asdict
 from datetime import datetime
 from xxhash import xxh64
 
 from typing import Union, Sequence, Mapping
-
 from .errors import PinsVersionError
 from ._types import StrOrFile, IOBase
 
+_log = logging.getLogger(__name__)
+
 VERSION_TIME_FORMAT = "%Y%m%dT%H%M%SZ"
 
 
@@ -116,3 +119,31 @@ def guess_version(x: str):
         return Version.from_string(x)
     except PinsVersionError:
         return VersionRaw(x)
+
+
+def version_setup(board, name, new_version, versioned):
+    if board.pin_exists(name):
+        versions_df = board.pin_versions(name, as_df=True)
+        versions = versions_df["version"].to_list()
+        old_version = versions[-1]
+        n_versions = len(versions)
+
+    else:
+        n_versions = 0
+
+    # if pin does not have version specified, see if multiple pins on board/board's version
+    if versioned is None:
+        versioned = True if n_versions > 1 else board.versioned
+
+    if versioned or n_versions == 0:
+        _log.info(f"Creating new version '{new_version}'")
+    elif n_versions == 1:
+        _log.info(f"Replacing version '{old_version}' with '{new_version}'")
+        board.pin_version_delete(name, old_version)
+    else:
+        raise PinsVersionError(
+            "Pin is versioned, but you have requested a write without versions."
+            "To un-version a pin, you must delete it"
+        )
+
+    return new_version