deid icon indicating copy to clipboard operation
deid copied to clipboard

CLI 'pixel' command

Open WillForan opened this issue 4 months ago • 3 comments

Minimal implementation of a clean-pixels command option for the CLI (#284).

  • deid/dicom/pixels/clean.py:_get_clean_name: refactor to allow no clean- prefix and no extension change for predicable dicom_save (new directory only, not new names). Not sure if that's wise -- just how I'd originally written the test and another user might expect output data.
  • deid/main/pixels.py: new file runs DicomCleaner detect, clean, save_dicom on input directory

I think it'd be useful to do both header and pixel scrubbing in one interface so everything described in a deid profile/config file can be applied at once. I'm not sure yet what the best way to go about that is (run identifiers into a new temp dir, rerun clean on that and save output to final location?)

WillForan avatar Sep 04 '25 15:09 WillForan

I think I'm all done with the random force pushes (sorry!). Anything still outstanding?

WillForan avatar Sep 09 '25 13:09 WillForan

This is looking good! Some final points I think:

  • Let's bump the version and add a note to the changelog
  • Can you confirm you've tested / like the behavior, etc?
  • There is redundancy in the argspec - e.g., I see deid and ids now for several commands. If possible, for the arguments you added for clean-pixels can you add them just once to the respective commands? I usually do:
# create command groups up here...

for command in [inspect, clean_pixels, etc]:
    # add the same argument here for deid, ids, etc.

Finally, we should have one more set of eyes, although the PR is fairly straight forward. Pinging @wetzelj if you are still around, but any contributor will do!

vsoch avatar Sep 11 '25 01:09 vsoch

(hopefully) fixed the redundant --deid and --input. confirmed command works as expect on a dataset in the wild as well as in test_cli.py with example deid-data

Incidentally, test_cli.py also caught a failed attempt to move --deid into the global parser instead of every subparser. Surprising to me: order is important and moving to global would break the existing specification (currently have eg. deid identifiers --deid deid.cfg but into global would force --deid before the sub command like deid --deid deid.cfg identifiers)

WillForan avatar Sep 28 '25 00:09 WillForan