Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 7, 2025

Summary

Removed redundant conditional check per code review feedback.

Changes Made

Modified: app.py (Line 200)

Removed the if parameters['tfLabelMap']: check before the cleanup loop. This check was redundant because:

  1. When tfLabelMap is empty, build_label_remapper returns a dict mapping src_labels to themselves - no invalid keys exist
  2. When tfLabelMap has only valid keys, the loop simply finds nothing to remove
  3. When tfLabelMap has invalid keys, the loop removes them

The loop works correctly in all three cases without the conditional check.

Code Change

# Before (with redundant check):
if parameters['tfLabelMap']:
    for key in list(label_remapper.keys()):
        if key not in src_label_set:
            label_remapper.pop(key)

# After (cleaner):
for key in list(label_remapper.keys()):
    if key not in src_label_set:
        label_remapper.pop(key)

Testing

  • ✅ Verified empty tfLabelMap case: no invalid keys to remove, loop does nothing
  • ✅ Verified valid keys only: no invalid keys to remove, loop does nothing
  • ✅ Verified invalid keys present: loop successfully removes them
  • ✅ Python syntax validation passed

The cleanup loop is now simpler and more straightforward while maintaining the same functionality.

Original prompt

This section details on the original issue you should resolve

<issue_title>catch bad user tfLabelMap</issue_title>
<issue_description>### New Feature Summary

As a user, if I put in a bad value for tfLabelMap (like one of my "in" labels does not match the available TimePoint) labels) I get an error message that gives me a clue about my mistake.

Related

This is a follow up of #145 . The messages in the Traceback don't give many clues about what caused the error. I didn't actually notice what I had done wrong.

Alternatives

Even just an assert statement to ensure that the user's labels were valid would be helpful!

Additional context

No response</issue_description>

<agent_instructions>Use metadata.py to find the labelSet value used in actual model output, compare the labelSet to "keys" from tfLabelMap dict passed as parameters and issue warning on the set diffs between two</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: keighrim <9062727+keighrim@users.noreply.github.com>
Copilot AI changed the title [WIP] Add error handling for bad user tfLabelMap input Validate tfLabelMap keys against actual TimePoint labels Nov 7, 2025
Copilot AI requested a review from keighrim November 7, 2025 21:39
@keighrim
Copy link
Member

keighrim commented Nov 7, 2025

@copilot Instead of raise an error and die, issue a runtime warning. Also, warning msg must show the bidirectional set diffs. Once warning is sent, check the values of label_remapper variable and pop out keys with zero-length values.

…apper

Co-authored-by: keighrim <9062727+keighrim@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot Instead of raise an error and die, issue a runtime warning. Also, warning msg must show the bidirectional set diffs. Once warning is sent, check the values of label_remapper variable and po...

Changed to runtime warning with bidirectional set diffs and label_remapper cleanup in commit f445638.

The warning now shows:

  • Invalid keys: labels in tfLabelMap not present in TimePoint labels
  • Unmapped keys: TimePoint labels not mapped in tfLabelMap

Invalid keys are removed from label_remapper after it's built, and processing continues instead of stopping.

Copilot AI changed the title Validate tfLabelMap keys against actual TimePoint labels Validate tfLabelMap keys against actual TimePoint labels with warnings Nov 7, 2025
@keighrim
Copy link
Member

keighrim commented Nov 7, 2025

@copilot revise

Co-authored-by: keighrim <9062727+keighrim@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot revise

Revised in commit be50ffc:

  • Warning messages now use "TimePoint labelSet" terminology
  • Neutral tone for unmapped keys (acknowledges user intention)
  • Explains consequences for both invalid and unmapped keys
  • Using .pop() on existing label_remapper instead of recreating it

Copilot AI changed the title Validate tfLabelMap keys against actual TimePoint labels with warnings Validate tfLabelMap keys against actual TimePoint labelSet with informative warnings Nov 7, 2025
Copilot AI requested a review from keighrim November 7, 2025 22:20
Co-authored-by: keighrim <9062727+keighrim@users.noreply.github.com>
@keighrim keighrim marked this pull request as ready for review November 7, 2025 22:29
@keighrim keighrim merged commit cec6889 into main Nov 7, 2025
@keighrim keighrim deleted the copilot/catch-bad-user-tflabelmap branch November 7, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

catch bad user tfLabelMap

2 participants