Repository: gtalarico/pyairtable
Pull request #347 (2024-03-14 00:26:42)

Return `""` and `False` from TextField, CheckboxField
It doesn't really make sense for a TextField or CheckboxField to return `None`, since Airtable will return `null` for a text field with an empty value or a checkbox field that is unchecked. This branch allows those fields to return empty strings or `False` as their "missing value", which will reduce the amount of defensive `if model.field is None: ...` style of type checking required by implementers.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/gtalarico/pyairtable/pull/347?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 99.65%. Comparing base [(`1633757`)](https://app.codecov.io/gh/gtalarico/pyairtable/commit/163375758f334d9a29e71c40975b61494e4b75ce?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico) to head [(`d1cd336`)](https://app.codecov.io/gh/gtalarico/pyairtable/pull/347?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico).

> :exclamation: Current head d1cd336 differs from pull request most recent head c7d5846. Consider uploading reports for the commit c7d5846 to get more accurate results


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   99.60%   99.65%   +0.05%     
==========================================
  Files          25       25              
  Lines        2025     2030       +5     
==========================================
+ Hits         2017     2023       +6     
+ Misses          8        7       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/gtalarico/pyairtable/pull/347?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Gui+Talarico).

mesozoic: Adding to 3.0 milestone since this would technically be a breaking API change

Patch:
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 08eceb5..8022d4d 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -5,6 +5,10 @@ Changelog
 3.0 (TBD)
 ------------------------
 
+* ORM fields :class:`~pyairtable.orm.fields.TextField` and
+  :class:`~pyairtable.orm.fields.CheckboxField` will no longer
+  return ``None`` when the field is empty.
+  - `PR #347 <https://github.com/gtalarico/pyairtable/pull/347>`_.
 * Rewrite of :mod:`pyairtable.formulas` module.
   - `PR #329 <https://github.com/gtalarico/pyairtable/pull/329>`_.
 
diff --git a/pyairtable/orm/fields.py b/pyairtable/orm/fields.py
index 3bf1805..2dbbffc 100644
--- a/pyairtable/orm/fields.py
+++ b/pyairtable/orm/fields.py
@@ -64,19 +64,22 @@ if TYPE_CHECKING:
 
 _ClassInfo: TypeAlias = Union[type, Tuple["_ClassInfo", ...]]
 T = TypeVar("T")
-T_Linked = TypeVar("T_Linked", bound="Model")
+T_Linked = TypeVar("T_Linked", bound="Model")  # used by LinkField
 T_API = TypeVar("T_API")  # type used to exchange values w/ Airtable API
 T_ORM = TypeVar("T_ORM")  # type used to store values internally
+T_Missing = TypeVar("T_Missing")  # type returned when Airtable has no value
 
 
-class Field(Generic[T_API, T_ORM], metaclass=abc.ABCMeta):
+class Field(Generic[T_API, T_ORM, T_Missing], metaclass=abc.ABCMeta):
     """
     A generic class for an Airtable field descriptor that will be
     included in an ORM model.
 
-    Type-checked subclasses should provide two type parameters,
-    ``T_API`` and ``T_ORM``, which indicate the type returned
-    by the API and the type used to store values internally.
+    Type-checked subclasses should provide three type parameters:
+
+        * ``T_API``, indicating the JSON-serializable type returned by the API
+        * ``T_ORM``, indicating the type used to store values internally
+        * ``T_Missing``, indicating the type of value returned if the field is empty
 
     Subclasses should also define ``valid_types`` as a type
     or tuple of types, which will be used to validate the type
@@ -149,11 +152,13 @@ class Field(Generic[T_API, T_ORM], metaclass=abc.ABCMeta):
 
     # obj.field will call __get__(instance=obj, owner=Model)
     @overload
-    def __get__(self, instance: "Model", owner: Type[Any]) -> Optional[T_ORM]: ...
+    def __get__(
+        self, instance: "Model", owner: Type[Any]
+    ) -> Union[T_ORM, T_Missing]: ...
 
     def __get__(
         self, instance: Optional["Model"], owner: Type[Any]
-    ) -> Union[SelfType, Optional[T_ORM]]:
+    ) -> Union[SelfType, T_ORM, T_Missing]:
         # allow calling Model.field to get the field object instead of a value
         if not instance:
             return self
@@ -173,8 +178,12 @@ class Field(Generic[T_API, T_ORM], metaclass=abc.ABCMeta):
     def __delete__(self, instance: "Model") -> None:
         raise AttributeError(f"cannot delete {self._description}")
 
-    def _missing_value(self) -> Optional[T_ORM]:
-        return None
+    @classmethod
+    def _missing_value(cls) -> T_Missing:
+        # This assumes Field[T_API, T_ORM, None]. If a subclass defines T_Missing as
+        # something different, it needs to override _missing_value.
+        # This can be tidied in 3.13 with T_Missing(default=None). See PEP-696.
+        return cast(T_Missing, None)
 
     def to_record_value(self, value: Any) -> Any:
         """
@@ -249,17 +258,36 @@ class Field(Generic[T_API, T_ORM], metaclass=abc.ABCMeta):
         return formulas.LTE(self, value)
 
 
-#: A generic Field whose internal and API representations are the same type.
-_BasicField: TypeAlias = Field[T, T]
+class _FieldWithTypedDefaultValue(Generic[T], Field[T, T, T]):
+    """
+    A generic Field with default value of the same type as internal and API representations.
+
+    For now this is used for TextField and CheckboxField, because Airtable stores the empty
+    values for those types ("" and False) internally as None.
+    """
+
+    @classmethod
+    def _missing_value(cls) -> T:
+        first_type = cls.valid_types
+        while isinstance(first_type, tuple):
+            if not first_type:
+                raise RuntimeError(f"{cls.__qualname__}.valid_types is malformed")
+            first_type = first_type[0]
+        return cast(T, first_type())
+
+
+#: A generic Field with internal and API representations that are the same type.
+_BasicField: TypeAlias = Field[T, T, None]
 
 
 #: An alias for any type of Field.
-AnyField: TypeAlias = _BasicField[Any]
+AnyField: TypeAlias = Field[Any, Any, Any]
 
 
-class TextField(_BasicField[str]):
+class TextField(_FieldWithTypedDefaultValue[str]):
     """
-    Used for all Airtable text fields. Accepts ``str``.
+    Accepts ``str``.
+    Returns ``""`` instead of ``None`` if the field is empty on the Airtable base.
 
     See `Single line text <https://airtable.com/developers/web/api/field-model#simpletext>`__
     and `Long text <https://airtable.com/developers/web/api/field-model#multilinetext>`__.
@@ -329,8 +357,9 @@ class RatingField(IntegerField):
             raise ValueError("rating cannot be below 1")
 
 
-class CheckboxField(_BasicField[bool]):
+class CheckboxField(_FieldWithTypedDefaultValue[bool]):
     """
+    Accepts ``bool``.
     Returns ``False`` instead of ``None`` if the field is empty on the Airtable base.
 
     See `Checkbox <https://airtable.com/developers/web/api/field-model#checkbox>`__.
@@ -338,11 +367,8 @@ class CheckboxField(_BasicField[bool]):
 
     valid_types = bool
 
-    def _missing_value(self) -> bool:
-        return False
 
-
-class DatetimeField(Field[str, datetime]):
+class DatetimeField(Field[str, datetime, None]):
     """
     DateTime field. Accepts only `datetime <https://docs.python.org/3/library/datetime.html#datetime-objects>`_ values.
 
@@ -364,7 +390,7 @@ class DatetimeField(Field[str, datetime]):
         return utils.datetime_from_iso_str(value)
 
 
-class DateField(Field[str, date]):
+class DateField(Field[str, date, None]):
     """
     Date field. Accepts only `date <https://docs.python.org/3/library/datetime.html#date-objects>`_ values.
 
@@ -386,7 +412,7 @@ class DateField(Field[str, date]):
         return utils.date_from_iso_str(value)
 
 
-class DurationField(Field[int, timedelta]):
+class DurationField(Field[int, timedelta, None]):
     """
     Duration field. Accepts only `timedelta <https://docs.python.org/3/library/datetime.html#timedelta-objects>`_ values.
 
@@ -418,7 +444,7 @@ class _DictField(Generic[T], _BasicField[T]):
     valid_types = dict
 
 
-class _ListField(Generic[T_API, T_ORM], Field[List[T_API], List[T_ORM]]):
+class _ListField(Generic[T_API, T_ORM], Field[List[T_API], List[T_ORM], List[T_ORM]]):
     """
     Generic type for a field that stores a list of values. Can be used
     to refer to a lookup field that might return more than one value.
@@ -455,11 +481,6 @@ class _ListField(Generic[T_API, T_ORM], Field[List[T_API], List[T_ORM]]):
                 instance._fields[self.field_name] = value
         return value
 
-    def to_internal_value(self, value: Optional[List[T_ORM]]) -> List[T_ORM]:
-        if value is None:
-            value = []
-        return value
-
 
 class _ValidatingListField(Generic[T], _ListField[T, T]):
     contains_type: Type[T]
@@ -825,13 +846,16 @@ class RichTextField(TextField):
     """
 
 
-class SelectField(TextField):
+class SelectField(Field[str, str, None]):
     """
-    Equivalent to :class:`~TextField`.
+    Represents a single select dropdown field. This will return ``None`` if no value is set,
+    and will only return ``""`` if an empty dropdown option is available and selected.
 
     See `Single select <https://airtable.com/developers/web/api/field-model#select>`__.
     """
 
+    valid_types = str
+
 
 class UrlField(TextField):
     """
@@ -923,10 +947,10 @@ import re
 with open(cog.inFile) as fp:
     src = fp.read()
 
-classes = re.findall(r"class ([A-Z]\w+Field)", src)
-constants = re.findall(r"^([A-Z][A-Z_+]) = ", src)
+classes = re.findall(r"class ((?:[A-Z]\w+)?Field)", src)
+constants = re.findall(r"^(?!T_)([A-Z][A-Z_]+) = ", src, re.MULTILINE)
 extras = ["LinkSelf"]
-names = sorted(classes + constants + extras)
+names = sorted(classes) + constants + extras
 
 cog.outl("\n\n__all__ = [")
 for name in ["Field", *names]:
@@ -953,12 +977,12 @@ __all__ = [
     "DurationField",
     "EmailField",
     "ExternalSyncSourceField",
+    "Field",
     "FloatField",
     "IntegerField",
     "LastModifiedByField",
     "LastModifiedTimeField",
     "LinkField",
-    "LinkSelf",
     "LookupField",
     "MultipleCollaboratorsField",
     "MultipleSelectField",
@@ -970,8 +994,13 @@ __all__ = [
     "SelectField",
     "TextField",
     "UrlField",
+    "ALL_FIELDS",
+    "READONLY_FIELDS",
+    "FIELD_TYPES_TO_CLASSES",
+    "FIELD_CLASSES_TO_TYPES",
+    "LinkSelf",
 ]
-# [[[end]]] (checksum: 3fa8c12315457baf170f9766fd8c9f8e)
+# [[[end]]] (checksum: 2aa36f4e76db73f3d0b741b6be6c9e9e)
 
 
 # Delayed import to avoid circular dependency
diff --git a/pyairtable/orm/model.py b/pyairtable/orm/model.py
index 21038cd..acfcdaa 100644
--- a/pyairtable/orm/model.py
+++ b/pyairtable/orm/model.py
@@ -286,10 +286,14 @@ class Model:
         # Convert Column Names into model field names
         field_values = {
             # Use field's to_internal_value to cast into model fields
-            field: name_field_map[field].to_internal_value(value)
+            field: (
+                name_field_map[field].to_internal_value(value)
+                if value is not None
+                else None
+            )
             for (field, value) in record["fields"].items()
             # Silently proceed if Airtable returns fields we don't recognize
-            if field in name_field_map and value is not None
+            if field in name_field_map
         }
         # Since instance(**field_values) will perform validation and fail on
         # any readonly fields, instead we directly set instance._fields.


Test patch:
diff --git a/tests/integration/test_integration_orm.py b/tests/integration/test_integration_orm.py
index 6300f34..4945592 100644
--- a/tests/integration/test_integration_orm.py
+++ b/tests/integration/test_integration_orm.py
@@ -219,7 +219,7 @@ def test_every_field(Everything):
 
     # The ORM won't refresh the model's field values after save()
     assert record.formula_integer is None
-    assert record.formula_nan is None
+    assert record.formula_nan == ""
     assert record.link_count is None
     assert record.lookup_error == []
     assert record.lookup_integer == []
diff --git a/tests/test_orm.py b/tests/test_orm.py
index ac90f14..4826d4f 100644
--- a/tests/test_orm.py
+++ b/tests/test_orm.py
@@ -15,7 +15,7 @@ from pyairtable.testing import fake_meta, fake_record
 class Address(Model):
     Meta = fake_meta(table_name="Address")
     street = f.TextField("Street")
-    number = f.TextField("Number")
+    number = f.IntegerField("Number")
 
 
 class Contact(Model):
@@ -73,7 +73,7 @@ def test_unsupplied_fields():
     """
     a = Address()
     assert a.number is None
-    assert a.street is None
+    assert a.street == ""
 
 
 def test_null_fields():
@@ -188,7 +188,7 @@ def test_linked_record_can_be_saved(requests_mock, access_linked_records):
     record IDs into instances of the model. This could interfere with save(),
     so this test ensures we don't regress the capability.
     """
-    address_json = fake_record(Number="123", Street="Fake St")
+    address_json = fake_record(Number=123, Street="Fake St")
     address_id = address_json["id"]
     address_url_re = re.escape(Address.get_table().url + "?filterByFormula=")
     contact_json = fake_record(Email="alice@example.com", Link=[address_id])
@@ -246,7 +246,7 @@ def test_undeclared_field(requests_mock, test_case):
     """
 
     record = fake_record(
-        Number="123",
+        Number=123,
         Street="Fake St",
         City="Springfield",
         State="IL",
@@ -265,7 +265,7 @@ def test_undeclared_field(requests_mock, test_case):
 
     _, get_model_instance = test_case
     instance = get_model_instance(Address, record["id"])
-    assert instance.to_record()["fields"] == {"Number": "123", "Street": "Fake St"}
+    assert instance.to_record()["fields"] == {"Number": 123, "Street": "Fake St"}
 
 
 @mock.patch("pyairtable.Table.batch_create")
@@ -275,19 +275,19 @@ def test_batch_save(mock_update, mock_create):
     Test that we can pass multiple unsaved Model instances (or dicts) to batch_save
     and it will create or update them all in as few requests as possible.
     """
-    addr1 = Address(number="123", street="Fake St")
-    addr2 = Address(number="456", street="Fake St")
+    addr1 = Address(number=123, street="Fake St")
+    addr2 = Address(number=456, street="Fake St")
     addr3 = Address.from_record(
         {
             "id": "recExistingRecord",
             "createdTime": datetime.utcnow().isoformat(),
-            "fields": {"Number": "789", "Street": "Fake St"},
+            "fields": {"Number": 789, "Street": "Fake St"},
         }
     )
 
     mock_create.return_value = [
-        fake_record(id="abc", Number="123", Street="Fake St"),
-        fake_record(id="def", Number="456", Street="Fake St"),
+        fake_record(id="abc", Number=123, Street="Fake St"),
+        fake_record(id="def", Number=456, Street="Fake St"),
     ]
 
     # Just like model.save(), Model.batch_save() will set IDs on new records.
@@ -298,8 +298,8 @@ def test_batch_save(mock_update, mock_create):
 
     mock_create.assert_called_once_with(
         [
-            {"Number": "123", "Street": "Fake St"},
-            {"Number": "456", "Street": "Fake St"},
+            {"Number": 123, "Street": "Fake St"},
+            {"Number": 456, "Street": "Fake St"},
         ],
         typecast=True,
     )
@@ -307,7 +307,7 @@ def test_batch_save(mock_update, mock_create):
         [
             {
                 "id": "recExistingRecord",
-                "fields": {"Number": "789", "Street": "Fake St"},
+                "fields": {"Number": 789, "Street": "Fake St"},
             },
         ],
         typecast=True,
@@ -366,8 +366,8 @@ def test_batch_delete__unsaved_record(mock_delete):
     receives any models which have not been created yet.
     """
     addresses = [
-        Address.from_record(fake_record(Number="1", Street="Fake St")),
-        Address(number="2", street="Fake St"),
+        Address.from_record(fake_record(Number=1, Street="Fake St")),
+        Address(number=2, street="Fake St"),
     ]
     with pytest.raises(ValueError):
         Address.batch_delete(addresses)
diff --git a/tests/test_orm_fields.py b/tests/test_orm_fields.py
index fd14d6f..c93c284 100644
--- a/tests/test_orm_fields.py
+++ b/tests/test_orm_fields.py
@@ -1,6 +1,7 @@
 import datetime
 import operator
 import re
+from unittest import mock
 
 import pytest
 
@@ -679,3 +680,77 @@ def test_datetime_timezones(requests_mock):
     obj.dt = datetime.datetime(2024, 3, 1, 11, 22, 33)
     obj.save()
     assert m.last_request.json()["fields"]["dt"] == "2024-03-01T11:22:33.000"
+
+
+@pytest.mark.parametrize(
+    "classinfo,expected",
+    [
+        (str, ""),
+        ((str, bool), ""),
+        ((((str,),),), ""),
+        (bool, False),
+    ],
+)
+def test_missing_value(classinfo, expected):
+    """
+    Test that _FieldWithTypedDefaultValue._missing_value finds the first
+    valid type and calls it to create the "missing from Airtable" value.
+    """
+
+    class F(f._FieldWithTypedDefaultValue):
+        valid_types = classinfo
+
+    class T:
+        the_field = F("Field Name")
+
+    assert T().the_field == expected
+
+
+@pytest.mark.parametrize(
+    "classinfo,exc_class",
+    [
+        ((), RuntimeError),
+        ((((), str), bool), RuntimeError),
+    ],
+)
+def test_missing_value__invalid_classinfo(classinfo, exc_class):
+    """
+    Test that _FieldWithTypedDefaultValue._missing_value raises an exception
+    if the class's valid_types is set to an invalid value.
+    """
+
+    class F(f._FieldWithTypedDefaultValue):
+        valid_types = classinfo
+
+    class T:
+        the_field = F("Field Name")
+
+    obj = T()
+    with pytest.raises(exc_class):
+        obj.the_field
+
+
+@pytest.mark.parametrize(
+    "fields,expected",
+    [
+        ({}, None),
+        ({"Field": None}, None),
+        ({"Field": ""}, ""),
+        ({"Field": "xyz"}, "xyz"),
+    ],
+)
+def test_select_field(fields, expected):
+    """
+    Test that select field distinguishes between empty string and None.
+    """
+
+    class T(Model):
+        Meta = fake_meta()
+        the_field = f.SelectField("Field")
+
+    obj = T.from_record(fake_record(**fields))
+    assert obj.the_field == expected
+
+    with mock.patch("pyairtable.Table.update", return_value=obj.to_record()) as m:
+        obj.save()
+        m.assert_called_once_with(obj.id, fields, typecast=True)
diff --git a/tests/test_typing.py b/tests/test_typing.py
index f8f0f2c..6001944 100644
--- a/tests/test_typing.py
+++ b/tests/test_typing.py
@@ -71,7 +71,7 @@ if TYPE_CHECKING:
         name = orm.fields.TextField("Name")
         logins = orm.fields.MultipleCollaboratorsField("Logins")
 
-    assert_type(Actor().name, Optional[str])
+    assert_type(Actor().name, str)
     assert_type(Actor().logins, List[T.CollaboratorDict])
 
     class Movie(orm.Model):
@@ -81,11 +81,11 @@ if TYPE_CHECKING:
         actors = orm.fields.LinkField("Actors", Actor)
 
     movie = Movie()
-    assert_type(movie.name, Optional[str])
+    assert_type(movie.name, str)
     assert_type(movie.rating, Optional[int])
     assert_type(movie.actors, List[Actor])
     assert_type(movie.prequels, List[Movie])
-    assert_type(movie.actors[0].name, Optional[str])
+    assert_type(movie.actors[0].name, str)
 
     class EveryField(orm.Model):
         aitext = orm.fields.AITextField("AI Generated Text")
@@ -123,7 +123,7 @@ if TYPE_CHECKING:
     assert_type(record.autonumber, Optional[int])
     assert_type(record.barcode, Optional[T.BarcodeDict])
     assert_type(record.button, Optional[T.ButtonDict])
-    assert_type(record.checkbox, Optional[bool])
+    assert_type(record.checkbox, bool)
     assert_type(record.collaborator, Optional[T.CollaboratorDict])
     assert_type(record.count, Optional[int])
     assert_type(record.created_by, Optional[T.CollaboratorDict])
@@ -132,7 +132,7 @@ if TYPE_CHECKING:
     assert_type(record.date, Optional[datetime.date])
     assert_type(record.datetime, Optional[datetime.datetime])
     assert_type(record.duration, Optional[datetime.timedelta])
-    assert_type(record.email, Optional[str])
+    assert_type(record.email, str)
     assert_type(record.float, Optional[float])
     assert_type(record.integer, Optional[int])
     assert_type(record.last_modified_by, Optional[T.CollaboratorDict])
@@ -141,8 +141,8 @@ if TYPE_CHECKING:
     assert_type(record.multi_select, List[str])
     assert_type(record.number, Optional[Union[int, float]])
     assert_type(record.percent, Optional[Union[int, float]])
-    assert_type(record.phone, Optional[str])
+    assert_type(record.phone, str)
     assert_type(record.rating, Optional[int])
-    assert_type(record.rich_text, Optional[str])
+    assert_type(record.rich_text, str)
     assert_type(record.select, Optional[str])
-    assert_type(record.url, Optional[str])
+    assert_type(record.url, str)