Warn on implicit --in-place

[blairconrad]

I've come to recommend against in-place anonymization in most cases. By overwriting the original files, we run the risk of losing those files. This is obvious, and can be protected against by copying the originals before anonymizing. A more subtle problem is that anonymization of dates in dicognito is performed by shifting them slightly into the past. If the same files are anonymized in place repeatedly, we find that study dates, patient birth dates, and the like continue to shift backwards in time, which can cause problems when the data is used.

Thus, dicognito will eventually require explicit choice of --in-place mode or a designated --output-directory.

[blairconrad]

@paulsbduncan, I'm bothering you for a review (no rush! take weeks) not of the code, although that would be welcome as well, but of the concept. You're a principal user. Is the end goal (require --output-directory or --in-place) reasonable? Can you think of a better goal? If this goal is good, is the path I'm taking reasonable?

[paulsbduncan]

I think so, @blairconrad. There are two main use cases I see:

1. I downloaded some real data to debug an issue at a site! Quick, anonymize it!
2. I have some 'seed' data and want to create more data

I want to make sure #1 is easy, because I think it is the most important case (though likely not the most USED case). Users should never be confused, thinking they've anonymized data when they've actually just made a copy of anonymized data.

So, I DO like the overall goal of requiring -o or -i (*). as long as it clearly communicates that -o doesn't anonymize the in place data.

*Oh, I'd love a short command for --in-place, like -i

[blairconrad]

Thanks for the feedback, @paulsbduncan. Yeah, I'm also hoping to avoid confusion. Hence the required switch rather than "well you didn't say --in-place, so I'll assume your first argument is the output directory".

I'll continue down this path.

*Oh, I'd love a short command for --in-place, like -i

I was going to say that we shouldn't've used -i as the short form for --id-prefix, but I guess we didn't. -p. Who knew! I guess -i was my proposed short form for the hypothetical "--id-infix" addition…

Your request is reasonable! PR to be updated as time allows.

[blairconrad]

I just noticed this part of your comment, @paulsbduncan

as long as it clearly communicates that -o doesn't anonymize the in place data

It's hard for the author to determine whether such clear communication is in place.

From the --help

  --output-directory OUTPUT_DIRECTORY, -o OUTPUT_DIRECTORY
                        Write anonymized files to OUTPUT_DIRECTORY. The output filename will be the
                        new SOP Instance UID. OUTPUT_DIRECTORY will be created if necessary.

I think this meets your goal. Tell me if it don't.

[paulsbduncan]

Yeah, I think your usage instructions are good. I think I was thinking of something drastic like a warning after you run it that the original files remain unanonymized, but probably that is going to far.

[blairconrad]

I think I was thinking of something drastic like a warning after you run it that the original files remain unanonymized, but probably that is going to far.

Agreed:

User: *uses tool in a perfectly cromulent way*
Tool: DANGER!! I DID WHAT YOU WANTED!

Thanks for the feedback!