Fix custom image subclass transforms#1400
Fix custom image subclass transforms#1400siddharth10ss wants to merge 8 commits intoTorchIO-project:mainfrom
Conversation
Fixes TorchIO-project#1391 Problem: Spatial transforms failed with custom Image subclasses that had non-standard __init__ parameters. Transforms used type(image)(...) which only passed standard parameters, causing TypeError for subclasses requiring additional arguments. Solution: Added new_like() factory method to Image class that: - Creates new image instances while preserving essential attributes - Can be overridden by subclasses to handle custom parameters - Maintains full backward compatibility Changes: - src/torchio/data/image.py: Add new_like() method - src/torchio/transforms/preprocessing/spatial/crop.py: Use new_like() - src/torchio/transforms/preprocessing/spatial/to_reference_space.py: Use new_like() - tests/transforms/test_custom_image_subclass.py: Add 8 comprehensive tests All new tests pass (8/8). Backward compatibility verified.
…al transforms - Add fallback mechanism to new_like() using copy.deepcopy() for robust custom subclass support - Update Pad, Resample, ToOrientation, Transpose transforms to use new_like() pattern - Add subject.update_attributes() calls for proper Subject synchronization - Add comprehensive test suite with 12 test cases covering all scenarios - Ensure ALL spatial transforms support custom Image subclasses with additional constructor parameters Key improvements: - Fallback handles subclasses without new_like() override automatically - Fixed Subject attribute/dictionary synchronization issue - Complete coverage of spatial transforms: Crop, Pad, CropOrPad, Resample, ToOrientation, Transpose, ToReferenceSpace - Maintains full backward compatibility with existing code Fixes TorchIO-project#1391 completely - all spatial transforms now work with custom Image subclasses
for more information, see https://pre-commit.ci
- Fix import order in test files: pytest before torch, blank line before local imports - Remove unused 'reference' variable in test_to_reference_space_with_custom_image - Addresses pre-commit.ci formatting issues
- Merge automatic formatting fixes from pre-commit.ci - Resolve conflict by keeping cleaner version without unused variable - All formatting issues now resolved
There was a problem hiding this comment.
Pull request overview
This PR implements a new_like() factory method in the base Image class to support custom Image subclasses with different constructor signatures. The issue arose when spatial transforms tried to reconstruct images using type(image)(tensor=..., affine=...), which failed for custom subclasses requiring additional parameters.
Changes:
- Added
new_like()method to the baseImageclass with a fallback mechanism usingcopy.deepcopy()for subclasses that don't override it - Updated all spatial transforms (Crop, Pad, Resample, ToOrientation, Transpose, ToReferenceSpace) to use
new_like()instead of direct constructor calls - Added comprehensive test coverage with two test files containing 12+ test cases covering various scenarios
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/torchio/data/image.py | Added new_like() factory method with try-except fallback mechanism and comprehensive documentation |
| src/torchio/transforms/preprocessing/spatial/crop.py | Updated to use new_like() for creating cropped images |
| src/torchio/transforms/preprocessing/spatial/pad.py | Updated to use new_like() and added subject.update_attributes() call |
| src/torchio/transforms/preprocessing/spatial/resample.py | Updated to use new_like() and added subject.update_attributes() call |
| src/torchio/transforms/preprocessing/spatial/to_orientation.py | Updated to use new_like() and added subject.update_attributes() call |
| src/torchio/transforms/preprocessing/spatial/transpose.py | Updated to use new_like() and added subject.update_attributes() call |
| src/torchio/transforms/preprocessing/spatial/to_reference_space.py | Updated build_image_from_reference() to use new_like() and added subject.update_attributes() call |
| tests/transforms/test_custom_image_subclass.py | Comprehensive test suite for custom Image subclasses with various transforms |
| tests/transforms/test_custom_image_subclass_fix.py | Additional test suite with similar coverage, potentially duplicating the other test file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hadowing - Remove duplicate test_custom_image_subclass_fix.py (keeping test_custom_image_subclass.py) - Fix variable shadowing in build_image_from_reference function - Use downsampled_reference instead of shadowing reference parameter
Update: All Feedback AddressedI've completed the changes requested in the Copilot code review in commit Changes Made:
|
|
Hi @siddharth10ss, thanks for working on this! The new_like() factory method is a good pattern, and your PR correctly identified the problem and the solution direction. I've opened a separate PR with a more minimal implementation: Here's why, and what's different: The core insight is that only 2 call sites actually reconstruct images via type(image)(...):
The other transforms (Pad, Resample, ToOrientation, Transpose, ToReferenceSpace.apply_transform) use in-place mutation (image.set_data() + image.affine = ...), which already works fine for custom subclasses — What the new PR keeps from yours:
What's different:
Thanks again for the initiative — your analysis of the problem was spot on, and the new_like idea from both you and the @tcollins-hub is the right call. |
|
Closing in favor of #1445. |
Fixes #1391
Summary
Implements the
new_like()factory method to support customImagesubclasses with different constructor signatures, with comprehensive fallback mechanism and full spatial transform coverage.Description
TorchIO spatial transforms previously reconstructed images using
type(image)(tensor=..., affine=...), which failed for customImagesubclasses with different__init__signatures (e.g., requiring additional parameters likehistory). This PR solves the issue by:new_like()method to the baseImageclass as a factory method for creating new instancescopy.deepcopy()for subclasses that don't overridenew_like()Changes
Core Implementation (
src/torchio/data/image.py)new_like(tensor, affine=None)method to baseImageclasscopy.deepcopy()for robust subclass supportTransform Updates
Updated all spatial transforms to use
new_like():Crop(src/torchio/transforms/preprocessing/spatial/crop.py)Pad(src/torchio/transforms/preprocessing/spatial/pad.py)CropOrPad(usesCropinternally, works automatically)Resample(src/torchio/transforms/preprocessing/spatial/resample.py)ToOrientation(src/torchio/transforms/preprocessing/spatial/to_orientation.py)Transpose(src/torchio/transforms/preprocessing/spatial/transpose.py)ToReferenceSpace(src/torchio/transforms/preprocessing/spatial/to_reference_space.py)Added
subject.update_attributes()calls for proper Subject-Image synchronization.Test Suite (
tests/transforms/preprocessing/test_image_factory.py)12 comprehensive test cases:
Example Usage