fix: adaptor map_names calls dict as function when inputs is a name-mapping dict#8907
fix: adaptor map_names calls dict as function when inputs is a name-mapping dict#8907aymuos15 wants to merge 3 commits into
Conversation
When adaptor wraps a function with **kwargs and inputs is a dict (name-mapping mode), map_names() called input_map(k, k) which treats the dict as a callable, raising TypeError. Use .get(k, k) to look up the mapping instead. Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
Tests that adaptor correctly renames keys when the wrapped function has **kwargs and inputs is a name-mapping dict. Fails before the fix with TypeError: 'dict' object is not callable. Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
📝 WalkthroughWalkthroughFixed the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
monai/transforms/adaptors.py (1)
142-143: 💤 Low valueConsider adding a docstring to the helper.
Per guidelines, all definitions should have docstrings describing parameters and return values, even for internal helpers.
📝 Suggested docstring
def map_names(ditems, input_map): + """Remap dictionary keys using input_map, falling back to original keys. + + Args: + ditems: Dictionary to remap. + input_map: Mapping from original to target key names. + + Returns: + Dictionary with remapped keys. + """ return {input_map.get(k, k): v for k, v in ditems.items()}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@monai/transforms/adaptors.py` around lines 142 - 143, Add a docstring to the helper function map_names in monai/transforms/adaptors.py that concisely describes the parameters and return value: document that ditems is a dict of original key->value pairs, input_map is a mapping used to rename keys (with input_map.get(k, k) behavior), and the function returns a new dict with keys remapped; include parameter types and a brief example or note about fallback behavior when a key is not found in input_map.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@monai/transforms/adaptors.py`:
- Around line 142-143: Add a docstring to the helper function map_names in
monai/transforms/adaptors.py that concisely describes the parameters and return
value: document that ditems is a dict of original key->value pairs, input_map is
a mapping used to rename keys (with input_map.get(k, k) behavior), and the
function returns a new dict with keys remapped; include parameter types and a
brief example or note about fallback behavior when a key is not found in
input_map.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 625f7500-e5cb-4a3e-834b-04757c2cfb8e
📒 Files selected for processing (2)
monai/transforms/adaptors.pytests/transforms/test_adaptors.py
fix: adaptor map_names calls dict as function when inputs is a name-mapping dict
When
adaptorwraps a function with**kwargsandinputsis a dict, themap_nameshelper callsinput_map(k, k)instead ofinput_map.get(k, k), raisingTypeErrorbefore any data reaches the wrapped function. The fix usesinput_map.get(k, k)so the dict is looked up rather than called, falling back to the original key when no mapping exists.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.