-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Enhance/metatensor extraction #8754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,16 +37,19 @@ class ExtractDataKeyFromMetaKeyd(MapTransform): | |
| keys: keys to be transferred from meta to data | ||
| meta_key: the meta key where all the meta-data is stored | ||
| allow_missing_keys: don't raise exception if key is missing | ||
| image_only: if True, only extract metadata from MetaTensor images to avoid duplication | ||
|
|
||
| Example: | ||
| When the fastMRI dataset is loaded, "kspace" is stored in the data dictionary, | ||
| but the ground-truth image with the key "reconstruction_rss" is stored in the meta data. | ||
| In this case, ExtractDataKeyFromMetaKeyd moves "reconstruction_rss" to data. | ||
| For MetaTensor objects, setting image_only=True prevents extracting redundant metadata. | ||
| """ | ||
|
|
||
| def __init__(self, keys: KeysCollection, meta_key: str, allow_missing_keys: bool = False) -> None: | ||
| def __init__(self, keys: KeysCollection, meta_key: str, allow_missing_keys: bool = False, image_only: bool = False) -> None: | ||
| MapTransform.__init__(self, keys, allow_missing_keys) | ||
| self.meta_key = meta_key | ||
| self.image_only = image_only | ||
|
Comment on lines
+49
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for test files and test coverage related to ExtractDataKeyFromMetaKeyd
find . -type f -name "*test*dictionary*" | head -20Repository: Project-MONAI/MONAI Length of output: 45 🏁 Script executed: # Search for references to ExtractDataKeyFromMetaKeyd in the codebase
rg "ExtractDataKeyFromMetaKeyd" --type py -lRepository: Project-MONAI/MONAI Length of output: 113 🏁 Script executed: # Search for image_only parameter usage/tests
rg "image_only" --type py -B 2 -A 2Repository: Project-MONAI/MONAI Length of output: 50375 🏁 Script executed: # Examine the implementation file to understand the parameter
head -100 monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 4812 🏁 Script executed: # Look for test files in reconstruction directory
find . -path "*reconstruction*test*" -type f -name "*.py"Repository: Project-MONAI/MONAI Length of output: 485 🏁 Script executed: # Search for any references to ExtractDataKeyFromMetaKeyd in the entire codebase
rg "ExtractDataKeyFromMetaKeyd" -B 3 -A 3Repository: Project-MONAI/MONAI Length of output: 2000 🏁 Script executed: # Examine the specific transform file more carefully to see the full implementation
sed -n '40,100p' monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 3131 🏁 Script executed: # Search for any tests that reference ExtractDataKeyFromMetaKeyd
rg "ExtractDataKeyFromMetaKeyd" tests/Repository: Project-MONAI/MONAI Length of output: 45 🏁 Script executed: # Check if MetaTensor is imported in the dictionary.py file
head -30 monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 1342 🏁 Script executed: # Check the full imports and look for MetaTensor import
cat monai/apps/reconstruction/transforms/dictionary.py | head -50Repository: Project-MONAI/MONAI Length of output: 2417 🏁 Script executed: # Search for MetaTensor import in the file
rg "MetaTensor" monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 420 🏁 Script executed: # Check if MetaTensor needs to be imported - search where it's defined
rg "class MetaTensor" monai/Repository: Project-MONAI/MONAI Length of output: 129 🏁 Script executed: # Look at the full __call__ method to understand the context better
sed -n '50,80p' monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 1288 🏁 Script executed: # Check the entire dictionary.py file for MetaTensor imports
cat monai/apps/reconstruction/transforms/dictionary.py | grep -n "MetaTensor\|import"Repository: Project-MONAI/MONAI Length of output: 1175 🏁 Script executed: # Check if MetaTensor is used anywhere in monai.data that we can import from
rg "from monai.data" monai/apps/reconstruction/transforms/dictionary.pyRepository: Project-MONAI/MONAI Length of output: 45 🏁 Script executed: # Look at similar dictionary transforms to see how they handle MetaTensor
rg "MetaTensor" monai/transforms/io/dictionary.py -B 2 -A 2 | head -20Repository: Project-MONAI/MONAI Length of output: 45 🏁 Script executed: # Check what MetaTensor import is needed
rg "from.*MetaTensor" monai/transforms/ | head -5Repository: Project-MONAI/MONAI Length of output: 605 🏁 Script executed: # Verify that MetaTensor is referenced but not imported by checking if code would fail
python3 << 'EOF'
import ast
import sys
with open('monai/apps/reconstruction/transforms/dictionary.py', 'r') as f:
content = f.read()
# Simple check: look for MetaTensor in code but not in imports
has_metatensor_import = 'from monai.data.meta_tensor import MetaTensor' in content
has_metatensor_usage = 'isinstance(extracted_value, MetaTensor)' in content
print(f"MetaTensor imported: {has_metatensor_import}")
print(f"MetaTensor used in code: {has_metatensor_usage}")
print(f"Will cause NameError if image_only=True is used: {has_metatensor_usage and not has_metatensor_import}")
EOFRepository: Project-MONAI/MONAI Length of output: 173 🏁 Script executed: # Check if there are ANY tests for ExtractDataKeyFromMetaKeyd
find tests/ -type f -name "*.py" -exec grep -l "ExtractDataKeyFromMetaKeyd" {} \;Repository: Project-MONAI/MONAI Length of output: 45 Add missing MetaTensor import and implement unit tests for the MetaTensor is referenced in line 69 but not imported—the code will raise NameError when 🤖 Prompt for AI Agents |
||
|
|
||
| def __call__(self, data: Mapping[Hashable, NdarrayOrTensor]) -> dict[Hashable, Tensor]: | ||
| """ | ||
|
|
@@ -60,7 +63,12 @@ def __call__(self, data: Mapping[Hashable, NdarrayOrTensor]) -> dict[Hashable, T | |
| d = dict(data) | ||
| for key in self.keys: | ||
| if key in d[self.meta_key]: | ||
| d[key] = d[self.meta_key][key] # type: ignore | ||
| extracted_value = d[self.meta_key][key] | ||
| # When image_only is True, skip if the extracted value is a MetaTensor | ||
| # to preserve metadata associations | ||
| if self.image_only and isinstance(extracted_value, MetaTensor): | ||
| continue | ||
| d[key] = extracted_value # type: ignore | ||
|
Comment on lines
+66
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🐛 Proposed fixAdd the import after the existing +from monai.data.meta_tensor import MetaTensor🧰 Tools🪛 Ruff (0.15.1)[error] 69-69: Undefined name (F821) 🤖 Prompt for AI Agents
Comment on lines
+67
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silent If Additionally, the parameter name 🧰 Tools🪛 Ruff (0.15.1)[error] 69-69: Undefined name (F821) 🤖 Prompt for AI Agents |
||
| elif not self.allow_missing_keys: | ||
| raise KeyError( | ||
| f"Key `{key}` of transform `{self.__class__.__name__}` was missing in the meta data" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,11 @@ class TensorBoardHandler: | |
|
|
||
| def __init__(self, summary_writer: SummaryWriter | SummaryWriterX | None = None, log_dir: str = "./runs"): | ||
| if summary_writer is None: | ||
| if SummaryWriter is None: | ||
| raise RuntimeError( | ||
| "TensorBoardHandler requires tensorboard to be installed. " | ||
| "Please install it with: pip install tensorboard" | ||
| ) | ||
|
Comment on lines
+53
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fix: retain the success flag from 🐛 Proposed fixAt line 34, capture the boolean: - SummaryWriter, _ = optional_import("torch.utils.tensorboard", name="SummaryWriter")
+ SummaryWriter, _has_summary_writer = optional_import("torch.utils.tensorboard", name="SummaryWriter")Then in - if SummaryWriter is None:
+ if not _has_summary_writer:
raise RuntimeError(
"TensorBoardHandler requires tensorboard to be installed. "
"Please install it with: pip install tensorboard"
)🧰 Tools🪛 Ruff (0.15.1)[warning] 54-57: Avoid specifying long messages outside the exception class (TRY003) 🤖 Prompt for AI Agents |
||
| self._writer = SummaryWriter(log_dir=log_dir) | ||
| self.internal_writer = True | ||
| else: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring for
image_onlyinverts the actual behavior.Line 40 says "only extract metadata from MetaTensor images", but the real effect is the opposite: MetaTensor values are skipped (not extracted). The example note on line 46 is also vague. Consider:
📝 Committable suggestion
🤖 Prompt for AI Agents