Repository: rstudio/vetiver-python
Pull request #127 (2022-11-18 12:47:27)

ENH: Added Python version to `vetiver_pin_write`
### Changes
This adds the python system information to the metadata

### Note
Hey folks! I found your repo through PyData NYC and was interested in contributing mainly to the deployment aspect. This is a basic PR for one of the issues to get a hang of the project. 

### Testing:
~There is no UT :)~ It's inside another folder, my bad
Sample run:
```py
> cat test_v.py
from vetiver import mock, VetiverModel, VetiverAPI, vetiver_pin_write
from pins import board_temp

X, y = mock.get_mock_data()
model = mock.get_mock_model().fit(X, y)

v = VetiverModel(model, model_name='mock_model')

model_board = board_temp(versioned = True, allow_pickle_read = True)
vetiver_pin_write(model_board, v)

loaded_v = VetiverModel.from_pin(board=model_board, name='mock_model')
print(loaded_v.metadata)
```
```sh
Model Cards provide a framework for transparent, responsible reporting. 
 Use the vetiver `.qmd` Quarto template as a place to start, 
 with vetiver.model_card()
Writing pin:
Name: 'mock_model'
Version: 20230113T202728Z-1a387
VetiverMeta(user={}, version='20230113T202728Z-1a387', url=None, required_pkgs=['scikit-learn'], python_version=(3, 10, 6, 'final', 0))
/home/ganesh/os/mlops/vetiver-python/hostt.py:19: DeprecationWarning: argument for checking input data prototype has changed to check_prototype, from check_ptype
```

### Alternatively
We could add it to [_model_meta](https://github.com/rstudio/vetiver-python/blob/90cc5708b1fbf8fb2be4eb166a20117075887645/vetiver/meta.py#L1) to make it global? Seemed too invasive so wanted a second opinion

resolves: #115

isabelizimm: Welcome Ganesh! Thank you so much for this contribution, and it was great to meet you at PyData NYC. :) 

I think `_model_meta` is the correct spot for this, since the user section on a VetiverModel should be user-given metadata. I'm currently working on a metadata refactor in #126 that should make the addition of new fields a lot easier (and clean out the `user` field). Once that PR is in, let's look at adding `python_version` to `_model_meta`!
ganesh-k13: Ah that makes sense, thanks for the info! Will keep an eye out for that PR and rebase when done. Will keep looking around other stuff in the project meanwhile :+1: 
isabelizimm: > Ah that makes sense, thanks for the info! Will keep an eye out for that PR and rebase when done. Will keep looking around other stuff in the project meanwhile 👍

Hi Ganesh-- the other PR (#126) has been merged! It should be easier to add the Python version to the metadata dataclass now :) 
ganesh-k13: Thanks! Sorry was not active, had a few ideas but got caught up. I'll try to finish it this week. 

Patch:
diff --git a/vetiver/meta.py b/vetiver/meta.py
index d2a10f9..a09e2e0 100644
--- a/vetiver/meta.py
+++ b/vetiver/meta.py
@@ -1,3 +1,4 @@
+import sys
 from dataclasses import dataclass, asdict, field
 from typing import Mapping
 
@@ -10,6 +11,7 @@ class VetiverMeta:
     version: "str | None" = None
     url: "str | None" = None
     required_pkgs: "list | None" = field(default_factory=list)
+    python_version: "tuple | None" = None
 
     def to_dict(self) -> Mapping:
         data = asdict(self)
@@ -25,9 +27,10 @@ class VetiverMeta:
         version = metadata.get("version", None)
         url = metadata.get("url", None)
         required_pkgs = metadata.get("required_pkgs", [])
+        python_version = tuple(metadata.get("python_version", sys.version_info))
 
         if pip_name:
             if not list(filter(lambda x: pip_name in x, required_pkgs)):
                 required_pkgs = required_pkgs + [f"{pip_name}"]
 
-        return cls(user, version, url, required_pkgs)
+        return cls(user, version, url, required_pkgs, python_version)
diff --git a/vetiver/pin_read_write.py b/vetiver/pin_read_write.py
index 8da5c87..db2b44e 100644
--- a/vetiver/pin_read_write.py
+++ b/vetiver/pin_read_write.py
@@ -70,6 +70,7 @@ def vetiver_pin_write(board, model: VetiverModel, versioned: bool = True):
             "vetiver_meta": {
                 "required_pkgs": model.metadata.required_pkgs,
                 "prototype": None if not model.prototype else model.prototype().json(),
+                "python_version": list(model.metadata.python_version),
             },
         },
         versioned=versioned,
diff --git a/vetiver/vetiver_model.py b/vetiver/vetiver_model.py
index 9cef84c..cfa7868 100644
--- a/vetiver/vetiver_model.py
+++ b/vetiver/vetiver_model.py
@@ -102,6 +102,7 @@ class VetiverModel:
         if "vetiver_meta" in meta.user:
             get_prototype = meta.user.get("vetiver_meta").get("prototype", None)
             required_pkgs = meta.user.get("vetiver_meta").get("required_pkgs", None)
+            python_version = meta.user.get("vetiver_meta").get("python_version", None)
             meta.user.pop("vetiver_meta")
         else:
             # ptype = meta.user.get("ptype", None)
@@ -113,6 +114,7 @@ class VetiverModel:
             #     get_prototype = None
 
             required_pkgs = meta.user.get("required_pkgs")
+            python_version = meta.user.get("python_version")
 
         return cls(
             model=model,
@@ -123,6 +125,7 @@ class VetiverModel:
                 "version": meta.version.version,
                 "url": meta.local.get("url"),  # None all the time, besides Connect,
                 "required_pkgs": required_pkgs,
+                "python_version": python_version,
             },
             prototype_data=json.loads(get_prototype) if get_prototype else None,
             versioned=True,


Test patch:
diff --git a/vetiver/tests/test_build_vetiver_model.py b/vetiver/tests/test_build_vetiver_model.py
index 2832ea0..354c548 100644
--- a/vetiver/tests/test_build_vetiver_model.py
+++ b/vetiver/tests/test_build_vetiver_model.py
@@ -1,4 +1,5 @@
 import sklearn
+import sys
 
 import vetiver as vt
 from vetiver.meta import VetiverMeta
@@ -113,6 +114,7 @@ def test_vetiver_model_use_ptype():
         version=None,
         url=None,
         required_pkgs=["scikit-learn"],
+        python_version=tuple(sys.version_info),
     )
 
 
@@ -137,5 +139,41 @@ def test_vetiver_model_from_pin():
     assert v2.metadata.user == {"test": 123}
     assert v2.metadata.version is not None
     assert v2.metadata.required_pkgs == ["scikit-learn"]
+    assert v2.metadata.python_version == tuple(sys.version_info)
+
+    board.pin_delete("model")
+
+
+def test_vetiver_model_from_pin_user_metadata():
+    """
+    Test if standard keys as part of :dataclass:`VetiverMeta` are picked
+    """
+    custom_meta = {
+        "test": 123,
+        "required_pkgs": ["foo", "bar"],
+        "python_version": [3, 10, 6, "final", 0],
+    }
+    loaded_pkgs = custom_meta["required_pkgs"] + ["scikit-learn"]
+
+    v = vt.VetiverModel(
+        model=model,
+        prototype_data=X_df,
+        model_name="model",
+        versioned=None,
+        description=None,
+        metadata=custom_meta,
+    )
+
+    board = pins.board_temp(allow_pickle_read=True)
+    vt.vetiver_pin_write(board=board, model=v)
+    v2 = vt.VetiverModel.from_pin(board, "model")
+
+    assert isinstance(v2, vt.VetiverModel)
+    assert isinstance(v2.model, sklearn.base.BaseEstimator)
+    assert isinstance(v2.prototype.construct(), pydantic.BaseModel)
+    assert v2.metadata.user == custom_meta
+    assert v2.metadata.version is not None
+    assert v2.metadata.required_pkgs == loaded_pkgs
+    assert v2.metadata.python_version == tuple(custom_meta["python_version"])
 
     board.pin_delete("model")