Removing state argument from DirectRow.delete_cell(s).

Factoring behavior out into a non-public helper _delete_cells()
and using that helper in both DirectRow and ConditionalRow.

Also making sure ConditionalRow.delete_cell(s) get tested (codepaths
were only previously traversed via the parent class).

Author: dhermes

gcloud/bigtable/row.py

@@ -217,48 +217,12 @@ def delete(self):
         """
         self._delete(state=None)
 
-    def delete_cell(self, column_family_id, column, time_range=None,
-                    state=None):
-        """Deletes cell in this row.
-
-        .. note::
-
-            This method adds a mutation to the accumulated mutations on this
-            :class:`Row`, but does not make an API request. To actually
-            send an API request (with the mutations) to the Google Cloud
-            Bigtable API, call :meth:`commit`.
-
-        :type column_family_id: str
-        :param column_family_id: The column family that contains the column
-                                 or columns with cells being deleted. Must be
-                                 of the form ``[_a-zA-Z0-9][-_.a-zA-Z0-9]*``.
-
-        :type column: bytes
-        :param column: The column within the column family that will have a
-                       cell deleted.
-
-        :type time_range: :class:`TimestampRange`
-        :param time_range: (Optional) The range of time within which cells
-                           should be deleted.
-
-        :type state: bool
-        :param state: (Optional) The state that the mutation should be
-                      applied in. Unset if the mutation is not conditional,
-                      otherwise :data:`True` or :data:`False`.
-        """
-        self.delete_cells(column_family_id, [column], time_range=time_range,
-                          state=state)
-
-    def delete_cells(self, column_family_id, columns, time_range=None,
-                     state=None):
-        """Deletes cells in this row.
-
-        .. note::
+    def _delete_cells(self, column_family_id, columns, time_range=None,
+                      state=None):
+        """Helper for :meth:`delete_cell` and :meth:`delete_cells`.
 
-            This method adds a mutation to the accumulated mutations on this
-            :class:`Row`, but does not make an API request. To actually
-            send an API request (with the mutations) to the Google Cloud
-            Bigtable API, call :meth:`commit`.
+        ``state`` is unused by :class:`DirectRow` but is used by
+        subclasses.
 
         :type column_family_id: str
         :param column_family_id: The column family that contains the column
@@ -268,17 +232,16 @@ def delete_cells(self, column_family_id, columns, time_range=None,
         :type columns: :class:`list` of :class:`str` /
                        :func:`unicode <unicode>`, or :class:`object`
         :param columns: The columns within the column family that will have
-                        cells deleted. If :attr:`Row.ALL_COLUMNS` is used then
+                        cells deleted. If :attr:`ALL_COLUMNS` is used then
                         the entire column family will be deleted from the row.
 
         :type time_range: :class:`TimestampRange`
         :param time_range: (Optional) The range of time within which cells
                            should be deleted.
 
         :type state: bool
-        :param state: (Optional) The state that the mutation should be
-                      applied in. Unset if the mutation is not conditional,
-                      otherwise :data:`True` or :data:`False`.
+        :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:
@@ -311,6 +274,60 @@ def delete_cells(self, column_family_id, columns, time_range=None,
             # processed without error.
             mutations_list.extend(to_append)
 
+    def delete_cell(self, column_family_id, column, time_range=None):
+        """Deletes cell in this row.
+
+        .. note::
+
+            This method adds a mutation to the accumulated mutations on this
+            row, but does not make an API request. To actually
+            send an API request (with the mutations) to the Google Cloud
+            Bigtable API, call :meth:`commit`.
+
+        :type column_family_id: str
+        :param column_family_id: The column family that contains the column
+                                 or columns with cells being deleted. Must be
+                                 of the form ``[_a-zA-Z0-9][-_.a-zA-Z0-9]*``.
+
+        :type column: bytes
+        :param column: The column within the column family that will have a
+                       cell deleted.
+
+        :type time_range: :class:`TimestampRange`
+        :param time_range: (Optional) The range of time within which cells
+                           should be deleted.
+        """
+        self._delete_cells(column_family_id, [column], time_range=time_range,
+                           state=None)
+
+    def delete_cells(self, column_family_id, columns, time_range=None):
+        """Deletes cells in this row.
+
+        .. note::
+
+            This method adds a mutation to the accumulated mutations on this
+            row, but does not make an API request. To actually
+            send an API request (with the mutations) to the Google Cloud
+            Bigtable API, call :meth:`commit`.
+
+        :type column_family_id: str
+        :param column_family_id: The column family that contains the column
+                                 or columns with cells being deleted. Must be
+                                 of the form ``[_a-zA-Z0-9][-_.a-zA-Z0-9]*``.
+
+        :type columns: :class:`list` of :class:`str` /
+                       :func:`unicode <unicode>`, or :class:`object`
+        :param columns: The columns within the column family that will have
+                        cells deleted. If :attr:`ALL_COLUMNS` is used then
+                        the entire column family will be deleted from the row.
+
+        :type time_range: :class:`TimestampRange`
+        :param time_range: (Optional) The range of time within which cells
+                           should be deleted.
+        """
+        self._delete_cells(column_family_id, columns, time_range=time_range,
+                           state=None)
+
     def commit(self):
         """Makes a ``MutateRow`` API request.
 
@@ -518,6 +535,70 @@ def delete(self, state=True):
                       applied in. Defaults to :data:`True`.
         """
         self._delete(state=state)
+
+    def delete_cell(self, column_family_id, column, time_range=None,
+                    state=True):
+        """Deletes cell in this row.
+
+        .. note::
+
+            This method adds a mutation to the accumulated mutations on this
+            row, but does not make an API request. To actually
+            send an API request (with the mutations) to the Google Cloud
+            Bigtable API, call :meth:`commit`.
+
+        :type column_family_id: str
+        :param column_family_id: The column family that contains the column
+                                 or columns with cells being deleted. Must be
+                                 of the form ``[_a-zA-Z0-9][-_.a-zA-Z0-9]*``.
+
+        :type column: bytes
+        :param column: The column within the column family that will have a
+                       cell deleted.
+
+        :type time_range: :class:`TimestampRange`
+        :param time_range: (Optional) The range of time within which cells
+                           should be deleted.
+
+        :type state: bool
+        :param state: (Optional) The state that the mutation should be
+                      applied in. Defaults to :data:`True`.
+        """
+        self._delete_cells(column_family_id, [column], time_range=time_range,
+                           state=state)
+
+    def delete_cells(self, column_family_id, columns, time_range=None,
+                     state=True):
+        """Deletes cells in this row.
+
+        .. note::
+
+            This method adds a mutation to the accumulated mutations on this
+            row, but does not make an API request. To actually
+            send an API request (with the mutations) to the Google Cloud
+            Bigtable API, call :meth:`commit`.
+
+        :type column_family_id: str
+        :param column_family_id: The column family that contains the column
+                                 or columns with cells being deleted. Must be
+                                 of the form ``[_a-zA-Z0-9][-_.a-zA-Z0-9]*``.
+
+        :type columns: :class:`list` of :class:`str` /
+                       :func:`unicode <unicode>`, or :class:`object`
+        :param columns: The columns within the column family that will have
+                        cells deleted. If :attr:`Row.ALL_COLUMNS` is used then
+                        the entire column family will be deleted from the row.
+
+        :type time_range: :class:`TimestampRange`
+        :param time_range: (Optional) The range of time within which cells
+                           should be deleted.
+
+        :type state: bool
+        :param state: (Optional) The state that the mutation should be
+                      applied in. Defaults to :data:`True`.
+        """
+        self._delete_cells(column_family_id, columns, time_range=time_range,
+                           state=state)
     # pylint: enable=arguments-differ
 
     def clear(self):

gcloud/bigtable/test_row.py

@@ -142,7 +142,7 @@ def __init__(self, *args, **kwargs):
                 self._kwargs = []
 
             # Replace the called method with one that logs arguments.
-            def delete_cells(self, *args, **kwargs):
+            def _delete_cells(self, *args, **kwargs):
                 self._args.append(args)
                 self._kwargs.append(kwargs)
 
@@ -414,8 +414,11 @@ def test_commit(self):
 
         row_key = b'row_key'
         table_name = 'projects/more-stuff'
-        column_family_id = u'column_family_id'
-        column = b'column'
+        column_family_id1 = u'column_family_id1'
+        column_family_id2 = u'column_family_id2'
+        column_family_id3 = u'column_family_id3'
+        column1 = b'column1'
+        column2 = b'column2'
         timeout_seconds = 262
         client = _Client(timeout_seconds=timeout_seconds)
         table = _Table(table_name, client=client)
@@ -426,20 +429,31 @@ def test_commit(self):
         value1 = b'bytes-value'
         mutation1 = data_pb2.Mutation(
             set_cell=data_pb2.Mutation.SetCell(
-                family_name=column_family_id,
-                column_qualifier=column,
+                family_name=column_family_id1,
+                column_qualifier=column1,
                 timestamp_micros=-1,  # Default value.
                 value=value1,
             ),
         )
         mutation2 = data_pb2.Mutation(
             delete_from_row=data_pb2.Mutation.DeleteFromRow(),
         )
+        mutation3 = data_pb2.Mutation(
+            delete_from_column=data_pb2.Mutation.DeleteFromColumn(
+                family_name=column_family_id2,
+                column_qualifier=column2,
+            ),
+        )
+        mutation4 = data_pb2.Mutation(
+            delete_from_family=data_pb2.Mutation.DeleteFromFamily(
+                family_name=column_family_id3,
+            ),
+        )
         request_pb = messages_pb2.CheckAndMutateRowRequest(
             table_name=table_name,
             row_key=row_key,
             predicate_filter=row_filter.to_pb(),
-            true_mutations=[mutation1],
+            true_mutations=[mutation1, mutation3, mutation4],
             false_mutations=[mutation2],
         )
 
@@ -455,8 +469,10 @@ def test_commit(self):
         expected_result = predicate_matched
 
         # Perform the method and check the result.
-        row.set_cell(column_family_id, column, value1, state=True)
+        row.set_cell(column_family_id1, column1, value1, state=True)
         row.delete(state=False)
+        row.delete_cell(column_family_id2, column2, state=True)
+        row.delete_cells(column_family_id3, row.ALL_COLUMNS, state=True)
         result = row.commit()
         self.assertEqual(result, expected_result)
         self.assertEqual(stub.method_calls, [(