0.2.29 rc #199
0.2.29 rc #199
Conversation
…ngelog update, added PyCharm to gitignore
updated pydicom dependency from 2.1.1 to 2.2.2
* #197 'remove and keep combo' - added tests that describe situation where remove overrules keep, and also explicitly show that except is using value as substring/regex match * #197 'remove and keep combo' - added quite straightforward "add fields marked with KEEP to skiplist", also fixed docstring on parser#perform_action and removed unused parameter, fixed typo in random place * #197 'remove and keep combo' - quotes changed as per code format convention * #197 'remove and keep combo' - changed keep list to be separate property, updated docs on action order, added two tests for ensuring blank action order * #197 'remove and keep combo' - changed keep fields collector to be more pythonic * #197 'remove and keep combo' - more safeguards in keep fields collector, clarified docs upon field that should not be None * #197 'remove and keep combo' - changelog update, typo fix, version bump
|
ack. Have a plan of testing new version in real env this/next week. |
|
ping @glebsts ! |
|
Hello there, sorry for being late, some focus shift. |
|
yep, that would work, and just make sure to document everything very clearly so there is no doubt about the functionality and order of things. Pinging @wetzelj to get his feedback on these changes! |
|
I've been keeping an eye on this issue/pr and am totally on board with the functionality changes/add. At the moment, we've been working on some other additions to our application that uses pydicom/deid, so we've stuck at 0.2.28, but intend to update to latest (and/or this rc) in the next month or so. |
|
Fantastic! So @glebsts go ahead if you want to PR your changes to this branch - we can review / discuss and then get that 0.0.29 finalized. |
|
@vsoch hey, just to clarify - which branch should I should I send PR to (and from which branch better to do it)? |
|
I think if we want it to extend the work here, it would make sense to PR TO 0.2.29-rc. Is there a better way you have in mind? |
|
no, just got lost a little bit. Thank you, will work now |
… fields being jittered or replaced to prevent them from being removed, tests, extended description of @keep property, changelog update, typo fix, version bump
Co-authored-by: Vanessasaurus <[email protected]>
#197 'remove and replace/jitter combo' - added once-evaluated list of…
|
I think we followed up here with a new PR, so its safe to close. Thanks everyone! |
|
Hi @vsoch - I think this PR was prematurely closed. When I first saw that this was closed, I did notice that #204 had been merged, but on closer review, #198 and #204 were merged into this PR, #204 wasn't a replacement PR that was merged into master. Since this was closed and not merged, these changes never made it into master (or subsequent branches). I am currently testing against PR #234. What do you think about the possibility of merging this into that PR? I still believe that there's definite merit to the changes that were introduced here. |
|
We sped past this particular release, so if someone wants to re-base and re-open, I'd be happy to review again! It's mostly an interest thing - 7 months with no response I figured it was outdated or not of interest, sorry about that. |
|
Sounds good. I'll work on it next week. Is it okay if I base it off of #234 or would you prefer it be based off of master? |
|
Whichever is easier to test! Both ultimately are going to be rebase off of master, so choose your pick. |
* #193 'bump pydicom' - bumped pydicom to 2.2.2, local tests green, changelog update, added PyCharm to gitignore * WIP 197 remove + keep combo should keep (#198) * #197 'remove and keep combo' - added tests that describe situation where remove overrules keep, and also explicitly show that except is using value as substring/regex match * #197 'remove and keep combo' - added quite straightforward "add fields marked with KEEP to skiplist", also fixed docstring on parser#perform_action and removed unused parameter, fixed typo in random place * #197 'remove and keep combo' - quotes changed as per code format convention * #197 'remove and keep combo' - changed keep list to be separate property, updated docs on action order, added two tests for ensuring blank action order * #197 'remove and keep combo' - changed keep fields collector to be more pythonic * #197 'remove and keep combo' - more safeguards in keep fields collector, clarified docs upon field that should not be None * #197 'remove and keep combo' - changelog update, typo fix, version bump * #197 'remove and replace/jitter combo' - added once-evaluated list of fields being jittered or replaced to prevent them from being removed, tests, extended description of @keep property, changelog update, typo fix, version bump * #197 'remove and replace/jitter combo' - run black * Update deid/version.py - agreed to bump to 0.3 * Bug with return of derive_ctp_coordinate Downstream code expects coordinate definitions to be a comma separated list of values, not a list of ints. * Update deid/dicom/parser.py Committing suggestion. * change derive_ctp_coordinates to private function Co-authored-by: Gleb <[email protected]> Co-authored-by: Vanessasaurus <[email protected]>
* add support for translating ctp coordinates * version bump * pin numpy and python 3.7, bump minor version * Add/remove keep behavior (Previous PR #199 - 0.2.29 rc) (#237) * #193 'bump pydicom' - bumped pydicom to 2.2.2, local tests green, changelog update, added PyCharm to gitignore * WIP 197 remove + keep combo should keep (#198) * #197 'remove and keep combo' - added tests that describe situation where remove overrules keep, and also explicitly show that except is using value as substring/regex match * #197 'remove and keep combo' - added quite straightforward "add fields marked with KEEP to skiplist", also fixed docstring on parser#perform_action and removed unused parameter, fixed typo in random place * #197 'remove and keep combo' - quotes changed as per code format convention * #197 'remove and keep combo' - changed keep list to be separate property, updated docs on action order, added two tests for ensuring blank action order * #197 'remove and keep combo' - changed keep fields collector to be more pythonic * #197 'remove and keep combo' - more safeguards in keep fields collector, clarified docs upon field that should not be None * #197 'remove and keep combo' - changelog update, typo fix, version bump * #197 'remove and replace/jitter combo' - added once-evaluated list of fields being jittered or replaced to prevent them from being removed, tests, extended description of @keep property, changelog update, typo fix, version bump * #197 'remove and replace/jitter combo' - run black * Update deid/version.py - agreed to bump to 0.3 * Bug with return of derive_ctp_coordinate Downstream code expects coordinate definitions to be a comma separated list of values, not a list of ints. * Update deid/dicom/parser.py Committing suggestion. * change derive_ctp_coordinates to private function Signed-off-by: vsoch <[email protected]> Co-authored-by: Gleb <[email protected]> Co-authored-by: Vanessasaurus <[email protected]> Signed-off-by: vsoch <[email protected]> Co-authored-by: wetzelj <[email protected]>
Description
@glebsts just opening this so when you have tested and are ready we can merge into master. Definitely no rush, we can do after the holiday! I just want to make sure I don't forget :)