Conversation
WalkthroughThe PR includes minor bug fixes to DICOM metadata handling, particularly correcting malformed string values and adding SR-specific metadata requirements. Dependencies are constrained, test fixtures are reformatted, and copyright years are updated. No public API signatures are altered. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
fc5c009 to
51063df
Compare
Signed-off-by: bluna301 <[email protected]>
…m typo fix Signed-off-by: bluna301 <[email protected]>
51063df to
a3a64b2
Compare
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
requirements-min.txt (1)
3-3: Consider tracking pkg_resources migration to importlib as separate technical debt work.The codebase uses
pkg_resources.working_setinmonai/deploy/utils/importutil.py(4 functions:is_dist_editable(),dist_module_path(),is_module_installed(),dist_requires()). While the setuptools <81.0.0 pin correctly addresses the immediate deprecation issue, migrating toimportlib.metadatawould eliminate this version constraint and is a worthwhile long-term improvement. However, this refactoring is best tracked as separate work since it requires code changes beyond the scope of the requirements file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@requirements-min.txt` at line 3, Create a technical-debt task to migrate uses of pkg_resources to importlib.metadata for the functions is_dist_editable(), dist_module_path(), is_module_installed(), and dist_requires() in monai/deploy/utils/importutil.py; the task should list the exact functions to change, describe replacing pkg_resources.working_set calls with importlib.metadata (or importlib_metadata for older Python), include tests to preserve current behavior (editable/install detection and requirement parsing), and note that once migration is complete the setuptools>=59.5.0,<81.0.0 pin in requirements-min.txt can be revisited or removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@requirements-min.txt`:
- Line 3: Create a technical-debt task to migrate uses of pkg_resources to
importlib.metadata for the functions is_dist_editable(), dist_module_path(),
is_module_installed(), and dist_requires() in monai/deploy/utils/importutil.py;
the task should list the exact functions to change, describe replacing
pkg_resources.working_set calls with importlib.metadata (or importlib_metadata
for older Python), include tests to preserve current behavior (editable/install
detection and requirement parsing), and note that once migration is complete the
setuptools>=59.5.0,<81.0.0 pin in requirements-min.txt can be revisited or
removed.



When preparing MAP pipelines for clinical integration, the DICOM SRs produced by the default
DICOMTextSRWriterOperatorwere validated using libraries such as dciodvfy and dicom-validator. The following validation logs are produced:The non-standard attributes that throw warnings are defined in write_common_modules within
dicom_utils.py; these are recognized in the source code as extensions of the DICOM IOD. This PR resolves the remaining warnings and errors.The
FileMetaInformationtags on the DICOM SR are written as follows (MicroDICOM viewer):This is manually set in
write_common_modules:However, this seems to encode the character
0and1to their UTF-8 byte values of0x30and0x31, respectively, which results in a value ofb"\x30\x31"-->30\31. This was updated to write the literal bytes00 01i.e.b"\x00\x01";FileMetaInformationGroupLengthsetting was also removed, as this will be handled automatically during DICOM writing.CompletionFlag indicates the degree of completeness of the DICOM SR. Considering the DICOM SR is complete in the context of the AI model (i.e. it is a finalized AI output from the model), this tag value was set to
COMPLETE.ReferencedPerformedProcedureStepSequence uniquely identifies the
Performed Procedure Step SOP Instancefor which the series is created. However, we are not tracking the Modality Performed Procedure Step (MPPS) via MAPs/MONAI Deploy; while this tag isType 3 (Optional)for most modalities, it is not copied bywrite_common_modules. As such, this tag value was written as an emptySequence.PerformedProcedureCodeSequence conveys the codes of the performed procedures pertaining to this SOP Instance. However, we are not tracking the Modality Performed Procedure Step (
MPPS) via MAPs/MONAI Deploy and this tag is not present for most imaging modalities. As such, this tag value was written as an emptySequence.Miscellaneous Updates:
seg_purpose_of_reference_code.CodeMeaning = '"Processing Algorithm'typo correctedsetuptools<81.0.0pin;pkg_resourcesis deprecated above this versiontestfilesSummary by CodeRabbit
Bug Fixes
Chores