CLI 'pixel' command#285
Conversation
new test file to confirm cli usage
compatibility, after, adds, removing
ff05ed4 to
981b1de
Compare
has_burn_in is never used, so dont save it 'import os' unnecessary
981b1de to
2e36cf3
Compare
72fa123 to
89bf7cb
Compare
This could be a breaking change if it `image_type='original'` was ever valid. I don't think it is (more below). `image_type` can/should likely be removed from all other save_* functions too. Ostensibly, the option existed to look into the original pixel_array. But `clean_pixel_data` is the only place I see with that variable and it is run as an isolated function w/ no access to the DicomCleaner object. DicomCleaner has no `original` attribute. ``` (Pdb) self.__dir__() ['font', 'cmap', 'output_folder', 'recipe', 'results', 'force', 'dicom_file', 'cleaned', '__module__', '__doc__', '__init__', 'default_font', 'detect', 'clean', 'get_figure', '_get_clean_name', 'save_png', 'save_animation', 'save_dicom', '__dict__', '__weakref__', '__new__', '__repr__', '__hash__', '__str__', '__getattribute__', '__setattr__', '__delattr__', '__lt__', '__le__', '__eq__', '__ne__', '__gt__', '__ge__', '__reduce_ex__', '__reduce__', '__getstate__', '__subclasshook__', '__init_subclass__', '__format__', '__sizeof__', '__dir__', '__class__'] (Pdb) self.original *** AttributeError: 'DicomCleaner' object has no attribute 'original' ````
a9aa79d to
910c5d8
Compare
cb32ca2 to
fe724de
Compare
|
I think I'm all done with the random force pushes (sorry!). Anything still outstanding? |
|
This is looking good! Some final points I think:
# 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! |
move subparser 'command' definitions together.
loop over commands to add '--deid' and '--input' where shared
* add comment justifying why --deid for all instead of in parser
(into `parser` would break existing code, order is important)
* make implicit `dest='command'` in subparser explicit
* replace 'action' with 'command' in subparser help and title
(avoid conflating with eg. `identifiers --action all`)
|
(hopefully) fixed the redundant Incidentally, |
Minimal implementation of a
clean-pixelscommand option for the CLI (#284).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.DicomCleanerdetect, clean, save_dicom on input directoryI 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?)