Addressed review feedback + fixed system tests.

Author: gkevinzheng

google/cloud/bigtable/row.py

@@ -210,18 +210,16 @@ def _delete_cells(self, column_family_id, columns, time_range=None, state=None):
         :param state: (Optional) The state that is passed along to
                       :meth:`_get_mutations`.
         """
-        mutations_list = self._get_mutations(state)
         if columns is self.ALL_COLUMNS:
             self._get_mutations(state).append(
                 mutations.DeleteAllFromFamily(family_to_delete=column_family_id)
             )
         else:
             start_timestamp_micros = None
             end_timestamp_micros = None
-            if time_range is not None:
-                timestamps = time_range._to_dict()
-                start_timestamp_micros = timestamps.get("start_timestamp_micros")
-                end_timestamp_micros = timestamps.get("end_timestamp_micros")
+            timestamps = time_range._to_dict() if time_range else {}
+            start_timestamp_micros = timestamps.get("start_timestamp_micros")
+            end_timestamp_micros = timestamps.get("end_timestamp_micros")
 
             to_append = []
             for column in columns:
@@ -237,7 +235,7 @@ def _delete_cells(self, column_family_id, columns, time_range=None, state=None):
 
             # We don't add the mutations until all columns have been
             # processed without error.
-            mutations_list.extend(to_append)
+            self._get_mutations(state).extend(to_append)
 
 
 class DirectRow(_SetDeleteRow):
@@ -973,9 +971,9 @@ def _parse_rmw_row_response(row_response):
     """
     result = {}
     for cell in row_response.cells:
-        result.setdefault(cell.family, {}).setdefault(cell.qualifier, []).append(
-            (cell.value, _datetime_from_microseconds(cell.timestamp_micros))
-        )
+        column_family = result.setdefault(cell.family, {})
+        column = column_family.setdefault(cell.qualifier, [])
+        column.append((cell.value, _datetime_from_microseconds(cell.timestamp_micros)))
     return result
 
 

tests/system/v2_client/test_data_api.py

@@ -1144,39 +1144,17 @@ def test_table_append_row_input_errors(data_table, rows_to_delete):
     rows_to_delete.append(data_table.direct_row(ROW_KEY))
 
     # Column names should be convertible to bytes (str or bytes)
-    with pytest.raises(TypeError):
+    with pytest.raises(AttributeError):
         row.append_cell_value(COLUMN_FAMILY_ID1, INT_COL_NAME, CELL_VAL1)
 
-    with pytest.raises(TypeError):
+    with pytest.raises(AttributeError):
         row.increment_cell_value(COLUMN_FAMILY_ID1, INT_COL_NAME, 1)
 
-    # Unicode for column name and value
-    with pytest.raises(UnicodeEncodeError):
-        row.append_cell_value(COLUMN_FAMILY_ID1, JOY_EMOJI, CELL_VAL1)
-
-    with pytest.raises(UnicodeEncodeError):
-        row.append_cell_value(COLUMN_FAMILY_ID1, COL_NAME1, JOY_EMOJI)
-
-    with pytest.raises(UnicodeEncodeError):
-        row.increment_cell_value(COLUMN_FAMILY_ID1, JOY_EMOJI, 1)
-
-    # Non-integer cell values for increment_cell_value
     with pytest.raises(ValueError):
         row.increment_cell_value(COLUMN_FAMILY_ID1, COL_NAME1, OVERFLOW_INT_CELL_VAL)
 
-    # increment_cell_value does not do input validation on the int_value, instead using
-    # proto-plus to do validation.
-    row.increment_cell_value(COLUMN_FAMILY_ID1, COL_NAME1, FLOAT_CELL_VAL)
-    row.increment_cell_value(COLUMN_FAMILY_ID1, COL_NAME2, FLOAT_CELL_VAL2)
-    row.commit()
-
-    row_data = data_table.read_row(ROW_KEY)
-    assert row_data.cells[COLUMN_FAMILY_ID1][COL_NAME1][0].value == int(
-        FLOAT_CELL_VAL
-    ).to_bytes(8, byteorder="big", signed=True)
-    assert row_data.cells[COLUMN_FAMILY_ID1][COL_NAME2][0].value == int(
-        FLOAT_CELL_VAL2
-    ).to_bytes(8, byteorder="big", signed=True)
+    with pytest.raises(TypeError):
+        row.increment_cell_value(COLUMN_FAMILY_ID1, COL_NAME1, FLOAT_CELL_VAL)
 
     # Can't have more than MAX_MUTATIONS mutations, but only enforced after
     # a row.commit

tests/unit/v2_client/test_row.py

@@ -303,13 +303,9 @@ def _delete_cells_helper(time_range=None):
 
     expected_mutation = DeleteRangeFromColumn(family=column_family_id, qualifier=column)
     if time_range is not None:
-        time_range_pb = time_range._to_pb()
-        if time_range_pb.start_timestamp_micros:
-            expected_mutation.start_timestamp_micros = (
-                time_range_pb.start_timestamp_micros
-            )
-        if time_range_pb.end_timestamp_micros:
-            expected_mutation.end_timestamp_micros = time_range_pb.end_timestamp_micros
+        timestamps = time_range._to_dict()
+        expected_mutation.start_timestamp_micros = timestamps.get("start_timestamp_micros")
+        expected_mutation.end_timestamp_micros = timestamps.get("end_timestamp_micros")
     _assert_mutations_equal(row._mutations, [expected_mutation])