Merge pull request #347 from mesozoic/orm_null_values

Return `""` and `False` from TextField, CheckboxField

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>`_.
 

pyairtable/orm/fields.py

@@ -64,19 +64,22 @@
 
 _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 @@ def __get__(self, instance: None, owner: Type[Any]) -> SelfType: ...
 
     # 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 @@ def __set__(self, instance: "Model", value: Optional[T_ORM]) -> None:
     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 @@ def lte(self, value: Any) -> "formulas.Comparison":
         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,20 +357,18 @@ def valid_or_raise(self, value: int) -> None:
             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>`__.
     """
 
     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 @@ def to_internal_value(self, value: 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 @@ def to_internal_value(self, value: 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 @@ def _get_list_value(self, instance: "Model") -> 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 @@ class UrlField(TextField):
 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 @@ class UrlField(TextField):
     "DurationField",
     "EmailField",
     "ExternalSyncSourceField",
+    "Field",
     "FloatField",
     "IntegerField",
     "LastModifiedByField",
     "LastModifiedTimeField",
     "LinkField",
-    "LinkSelf",
     "LookupField",
     "MultipleCollaboratorsField",
     "MultipleSelectField",
@@ -970,8 +994,13 @@ class UrlField(TextField):
     "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

pyairtable/orm/model.py

@@ -286,10 +286,14 @@ def from_record(cls, record: RecordDict) -> SelfType:
         # 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.

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 == []

tests/test_orm.py

@@ -15,7 +15,7 @@
 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="[email protected]", 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,16 +298,16 @@ 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,
     )
     mock_update.assert_called_once_with(
         [
             {
                 "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)