[RFC][caffe2] TaskGroup.__repr__ shouldn't have side effects

Summary: `__repr__` calling self.tasks() ends up marking the instance as "used", which doesn't seem appropriate. I was debugging a value being passed around and then ran into `Cannot add Task to an already used TaskGroup.` because the value had been logged once.

Test Plan:
Added a unit test -- didn't see a clean public method to test it, but I'm happy to add one if that makes sense.

Will wait for sandcastle to trigger everything else; I'm not at all familiar with this code so any other recommendations would be great!

Reviewed By: cryptopic

Differential Revision: D23541198

fbshipit-source-id: 5d1ec674a1ddaedf113140133b90e0da6afa7270
This commit is contained in:
Kunal Bhalla
2020-10-01 14:19:08 -07:00
committed by Facebook GitHub Bot
parent 03e4e94d24
commit 4564444c91
2 changed files with 8 additions and 10 deletions

View File

@ -1,10 +1,6 @@
## @package task
# Module caffe2.python.task
from caffe2.python import core, context
from caffe2.python.schema import Field, from_blob_list
from collections import defaultdict
@ -354,7 +350,9 @@ class TaskGroup(object):
def __repr__(self):
return "TaskGroup(tasks={}, workspace_type={}, remote_nets={})".format(
self.tasks(), self.workspace_type(), self.remote_nets())
self._tasks + self._tasks_to_add,
self.workspace_type(),
self.remote_nets())
class TaskOutput(object):

View File

@ -1,8 +1,3 @@
import unittest
from caffe2.python import task
@ -22,3 +17,8 @@ class TestTask(unittest.TestCase):
]
for obj, want in cases:
self.assertEqual(obj.__repr__(), want)
def testEffectlessRepr(self):
task_group = task.TaskGroup()
_repr = task_group.__repr__()
self.assertFalse(task_group._already_used)