diff --git a/python/google/protobuf/internal/containers.py b/python/google/protobuf/internal/containers.py index 20c6d982fb..23357816f6 100755 --- a/python/google/protobuf/internal/containers.py +++ b/python/google/protobuf/internal/containers.py @@ -136,17 +136,7 @@ class RepeatedScalarFieldContainer(BaseContainer[_T], MutableSequence[_T]): def extend(self, elem_seq: Iterable[_T]) -> None: """Extends by appending the given iterable. Similar to list.extend().""" -# TODO: Change OSS to raise error too - if elem_seq is None: - return - try: - elem_seq_iter = iter(elem_seq) - except TypeError: - if not elem_seq: - warnings.warn('Value is not iterable. Please remove the wrong ' - 'usage. This will be changed to raise TypeError soon.') - return - raise + elem_seq_iter = iter(elem_seq) new_values = [self._type_checker.CheckValue(elem) for elem in elem_seq_iter] if new_values: self._values.extend(new_values) diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index a6223c2a7e..85ecacefa2 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -1012,6 +1012,12 @@ class MessageTest(unittest.TestCase): m = message_module.TestAllTypes() self.assertSequenceEqual([], m.repeated_int32) + for falsy_value in MessageTest.FALSY_VALUES: + with self.assertRaises(TypeError) as context: + m.repeated_int32.extend(falsy_value) + self.assertIn('iterable', str(context.exception)) + self.assertSequenceEqual([], m.repeated_int32) + for empty_value in MessageTest.EMPTY_VALUES: m.repeated_int32.extend(empty_value) self.assertSequenceEqual([], m.repeated_int32) @@ -1021,6 +1027,12 @@ class MessageTest(unittest.TestCase): m = message_module.TestAllTypes() self.assertSequenceEqual([], m.repeated_float) + for falsy_value in MessageTest.FALSY_VALUES: + with self.assertRaises(TypeError) as context: + m.repeated_float.extend(falsy_value) + self.assertIn('iterable', str(context.exception)) + self.assertSequenceEqual([], m.repeated_float) + for empty_value in MessageTest.EMPTY_VALUES: m.repeated_float.extend(empty_value) self.assertSequenceEqual([], m.repeated_float) @@ -1030,6 +1042,12 @@ class MessageTest(unittest.TestCase): m = message_module.TestAllTypes() self.assertSequenceEqual([], m.repeated_string) + for falsy_value in MessageTest.FALSY_VALUES: + with self.assertRaises(TypeError) as context: + m.repeated_string.extend(falsy_value) + self.assertIn('iterable', str(context.exception)) + self.assertSequenceEqual([], m.repeated_string) + for empty_value in MessageTest.EMPTY_VALUES: m.repeated_string.extend(empty_value) self.assertSequenceEqual([], m.repeated_string) diff --git a/python/google/protobuf/pyext/repeated_scalar_container.cc b/python/google/protobuf/pyext/repeated_scalar_container.cc index cd8654eb8d..92724c5caa 100644 --- a/python/google/protobuf/pyext/repeated_scalar_container.cc +++ b/python/google/protobuf/pyext/repeated_scalar_container.cc @@ -443,20 +443,6 @@ static int AssSubscript(PyObject* pself, PyObject* slice, PyObject* value) { PyObject* Extend(RepeatedScalarContainer* self, PyObject* value) { cmessage::AssureWritable(self->parent); - // TODO: Remove this in OSS - if (value == Py_None) { - PyErr_Warn(nullptr, - "Value is not iterable. Please remove the wrong usage." - " This will be changed to raise TypeError soon."); - Py_RETURN_NONE; - } - if ((Py_TYPE(value)->tp_as_sequence == nullptr) && PyObject_Not(value)) { - PyErr_Warn(nullptr, - "Value is not iterable. Please remove the wrong usage." - " This will be changed to raise TypeError soon."); - Py_RETURN_NONE; - } - ScopedPyObjectPtr iter(PyObject_GetIter(value)); if (iter == nullptr) { PyErr_SetString(PyExc_TypeError, "Value must be iterable");