Converter return default instead of None (Recidiviz/recidiviz-data#884)

* converter return default instead of None

* remove optional from entity fields that are non-nullable

* change to have caller use full defaults

* Revert "change to have caller use full defaults"

This reverts commit 17bf9ca912fdf6ecb801aebb117e3b8b8947bf2d.

* fix bond status default

GitOrigin-RevId: a65ebd02c01f3d40c19c780c9e35f6863442ffea

recidiviz/persistence/converter/bond.py

@@ -32,17 +32,21 @@ def convert(proto, metadata: IngestMetadata) -> entities.Bond:
         converter_utils.parse_bond_amount_and_check_for_type_and_status_info,
         'amount',
         proto,
-        default=(None, None, BondStatus.UNKNOWN_FOUND_IN_SOURCE))
-    # |inferred_bond_type| defaults to CASH unless the bond amount contains
-    # information about the type (i.e. NO_BOND or BOND_DENIED)
-    new.bond_type = fn(BondType.parse, 'bond_type', proto,
-                       metadata.enum_overrides, default=inferred_bond_type)
+        default=(None, None, None))
+    # If there is a bond amount we infer the bond type is CASH unless the bond
+    # amount contains information about the type (i.e. NO_BOND or BOND_DENIED).
+    # If there is not a bond amount, the bond type defaults to None.
+    new.bond_type = fn(
+        BondType.parse, 'bond_type', proto, metadata.enum_overrides,
+        default=inferred_bond_type or None)
     new.bond_type_raw_text = fn(normalize, 'bond_type', proto)
 
-    # |inferred_bond_status| defaults to UNKNOWN_FOUND_IN_SOURCE unless the bond
-    # amount contains information about the status (i.e. BOND_DENIED)
-    new.status = fn(BondStatus.parse, 'status', proto,
-                    metadata.enum_overrides, default=inferred_status)
+    # The bond status is set to UNKNOWN_FOUND_IN_SOURCE unless a different bond
+    # status is provided or the bond amount contains information about the
+    # status (i.e. BOND_DENIED).
+    new.status = fn(
+        BondStatus.parse, 'status', proto, metadata.enum_overrides,
+        default=inferred_status or BondStatus.UNKNOWN_FOUND_IN_SOURCE)
     new.status_raw_text = fn(normalize, 'status', proto)
 
     # TODO(745): convert once field exists on proto

recidiviz/persistence/converter/converter_utils.py

@@ -33,13 +33,13 @@
 
 def fn(func, field_name, proto, *additional_func_args, default=None):
     """Return the result of applying the given function to the field on the
-    proto, returning |default| if the proto field is unset.
+    proto, returning |default| if the proto field is unset or the function
+    returns None.
     """
-    if not proto.HasField(field_name):
-        return default
-    if not additional_func_args:
-        return func(getattr(proto, field_name))
-    return func(getattr(proto, field_name), *additional_func_args)
+    value = None
+    if proto.HasField(field_name):
+        value = func(getattr(proto, field_name), *additional_func_args)
+    return value if value is not None else default
 
 
 def parse_external_id(id_str):

recidiviz/persistence/entities.py

@@ -55,7 +55,7 @@ class Person(Entity, BuildableAttr):
     gender_raw_text: Optional[str] = attr.ib()
     race: Optional[Race] = attr.ib()
     race_raw_text: Optional[str] = attr.ib()
-    region: Optional[str] = attr.ib()
+    region: str = attr.ib()  # non-nullable
     ethnicity: Optional[Ethnicity] = attr.ib()
     ethnicity_raw_text: Optional[str] = attr.ib()
     place_of_residence: Optional[str] = attr.ib()
@@ -75,12 +75,12 @@ class Booking(Entity, BuildableAttr):
     projected_release_date: Optional[datetime.date] = attr.ib()
     release_reason: Optional[ReleaseReason] = attr.ib()
     release_reason_raw_text: Optional[str] = attr.ib()
-    custody_status: Optional[CustodyStatus] = attr.ib()
+    custody_status: CustodyStatus = attr.ib()  # non-nullable
     custody_status_raw_text: Optional[str] = attr.ib()
     facility: Optional[str] = attr.ib()
     classification: Optional[Classification] = attr.ib()
     classification_raw_text: Optional[str] = attr.ib()
-    last_seen_time: datetime.datetime = attr.ib()
+    last_seen_time: datetime.datetime = attr.ib()  # non-nullable
 
     booking_id: Optional[int] = attr.ib(default=None)
     holds: List['Hold'] = attr.ib(factory=list)
@@ -112,7 +112,7 @@ class Charge(Entity, BuildableAttr):
     level: Optional[str] = attr.ib()
     fee_dollars: Optional[int] = attr.ib()
     charging_entity: Optional[str] = attr.ib()
-    status: Optional[ChargeStatus] = attr.ib()
+    status: ChargeStatus = attr.ib()  # non-nullable
     status_raw_text: Optional[str] = attr.ib()
     court_type: Optional[CourtType] = attr.ib()
     court_type_raw_text: Optional[str] = attr.ib()
@@ -129,7 +129,7 @@ class Charge(Entity, BuildableAttr):
 @attr.s
 class Hold(Entity, BuildableAttr):
     jurisdiction_name: Optional[str] = attr.ib()
-    status: Optional[HoldStatus] = attr.ib()
+    status: HoldStatus = attr.ib()  # non-nullable
     status_raw_text: Optional[str] = attr.ib()
 
     hold_id: Optional[int] = attr.ib(default=None)
@@ -140,7 +140,7 @@ class Bond(Entity, BuildableAttr):
     amount_dollars: Optional[int] = attr.ib()
     bond_type: Optional[BondType] = attr.ib()
     bond_type_raw_text: Optional[str] = attr.ib()
-    status: Optional[BondStatus] = attr.ib()
+    status: BondStatus = attr.ib()  # non-nullable
     status_raw_text: Optional[str] = attr.ib()
     bond_agent: Optional[str] = attr.ib()
 
@@ -150,7 +150,7 @@ class Bond(Entity, BuildableAttr):
 @attr.s
 class Sentence(Entity, BuildableAttr):
     sentencing_region: Optional[str] = attr.ib()
-    status: Optional[SentenceStatus] = attr.ib()
+    status: SentenceStatus = attr.ib()  # non-nullable
     status_raw_text: Optional[str] = attr.ib()
     min_length_days: Optional[int] = attr.ib()
     max_length_days: Optional[int] = attr.ib()

recidiviz/persistence/entity_matching_utils.py

@@ -21,6 +21,7 @@
 from typing import Callable, Optional
 
 from recidiviz.persistence import entities
+from recidiviz.common.constants.bond import BondStatus
 
 
 # '*' catches positional arguments, making our arguments named and required.
@@ -173,7 +174,7 @@ def is_bond_match(
 def _sanitize_bond(bond: entities.Bond) -> entities.Bond:
     sanitized = copy.deepcopy(bond)
     sanitized.bond_id = None
-    sanitized.status = None
+    sanitized.status = BondStatus.UNKNOWN_REMOVED_FROM_SOURCE
     return sanitized