Merge pull request #1050 from facebookresearch/clanup-todo-1008-comments

Clean up TODO comments

mephisto/abstractions/providers/prolific/api/constants.py

@@ -4,7 +4,7 @@
 # --- Studies ---
 
 # HACK: Hardcoded Question IDs (Prolific doesn't have a better way for now)
-# TODO (#1008): Make this dynamic as soon as possible
+# [Depends on Prolific] Make this dynamic as soon as possible
 ELIGIBILITY_REQUIREMENT_AGE_RANGE_QUESTION_ID = "54ac6ea9fdf99b2204feb893"
 
 # https://docs.prolific.co/docs/api-docs/public/#tag/Studies/The-study-object

mephisto/abstractions/providers/prolific/api/eligibility_requirement_classes/age_range_eligibility_requirement.py

@@ -20,7 +20,7 @@ def to_prolific_dict(self) -> dict:
  prolific_dict = super().to_prolific_dict()
 
  # HACK: Hardcoded Question IDs (Prolific doesn't have a better way for now)
- # TODO (#1008): Make this dynamic as soon as possible
+ # [Depends on Prolific] Make this dynamic as soon as possible
  prolific_dict["query"] = dict(id=ELIGIBILITY_REQUIREMENT_AGE_RANGE_QUESTION_ID)
 
  return prolific_dict

mephisto/abstractions/providers/prolific/prolific_agent.py

@@ -156,7 +156,7 @@ def reject_work(self, reason) -> None:
  prolific_study_id = self.unit.get_prolific_study_id()
  worker_id = self.worker.get_prolific_participant_id()
 
- # TODO (#1008): remove this suppression of exception when Prolific fixes their API
+ # [Depends on Prolific] remove this suppression of exception when Prolific fixes their API
  from .api.exceptions import ProlificException
 
  try:

mephisto/abstractions/providers/prolific/prolific_datastore.py

@@ -742,7 +742,7 @@ def register_run(
  frame_height: int = 0,
  actual_available_places: Optional[int] = None,
  listed_available_places: Optional[int] = None,
- prolific_study_id: Optional[str] = None, # TODO (#1008): Remove this, left it just in case
+ prolific_study_id: Optional[str] = None,
  ) -> None:
  """Register a new task run in the Task Runs table"""
  with self.table_access_condition, self._get_connection() as conn:

mephisto/abstractions/providers/prolific/prolific_unit.py

@@ -267,7 +267,7 @@ def get_pay_amount(self) -> float:
  def launch(self, task_url: str) -> None:
  """Publish this Unit on Prolific (making it available)"""
 
- # TODO (#1008): if we have `max_num_concurrent_units` specified,
+ # [Depends on Prolific] if we have `max_num_concurrent_units` specified,
  # the Study cannot be stopped from Prolific UI.
  # That's beceause Mephisto waits until "completed" (not "assigned") status of previous
  # units before launching new ones. So if Prolific temporarily runs out of available Units

mephisto/abstractions/providers/prolific/prolific_utils.py

@@ -420,7 +420,6 @@ def compose_completion_codes(code_suffix: str) -> List[dict]:
  logger.debug(f"Initial completion codes for creating Study: {completion_codes_random}")
 
  try:
- # TODO (#1008): Make sure that all parameters are correct
  study: Study = client.Studies.create(
  project=prolific_project_id,
  name=name,
@@ -685,8 +684,8 @@ def is_worker_blocked(
 ) -> bool:
  """Determine if the given worker is blocked by this client
 
- TODO (#1008): do we even need to check with Prolific "Blocked Participants" group
-  (as opposed to out datastore)? Because it doesn't reflect Prolific's internal banning
+ Note that we're currently not using this check against Prolific "Blocked Participants" group
+ and simply looked at `is_blocked` column in our datastore.
  """
  workspace = find_or_create_prolific_workspace(
  client,
@@ -779,7 +778,7 @@ def approve_work(
  logger.warning(f'No submission found for study "{study_id}" and participant "{worker_id}"')
  return None
 
- # TODO (#1008): Maybe we need to expand handling submission statuses
+ # TODO (#1008): Handle other statuses later (when Submission was reviewed in Prolific UI)
  if submission.status == constants.SubmissionStatus.AWAITING_REVIEW:
  try:
  submission: Submission = client.Submissions.approve(submission.id)
@@ -808,7 +807,7 @@ def reject_work(
  logger.warning(f'No submission found for study "{study_id}" and participant "{worker_id}"')
  return None
 
- # TODO (#1008): Maybe we need to expand handling submission statuses
+ # TODO (#1008): Handle other statuses later (when Submission was reviewed in Prolific UI)
  if submission.status == constants.SubmissionStatus.AWAITING_REVIEW:
  try:
  submission: Submission = client.Submissions.reject(submission.id)