Feature add config loader #35
Conversation
|
Here is an idea... akin to the cleaner, what are your thoughts on having this be a class? Then you could have easy functions for adding, viewing a filter, saving, etc. It would be a good start to (eventually) plug into tools to help the user create, edit, and view recipes. |
deid/dicom/header.py
Outdated
| @@ -46,7 +46,7 @@ | |||
| import dateutil.parser | |||
| import tempfile | |||
|
|
|||
| from .utils import get_func, save_dicom | |||
| from .utils import get_func, save_dicom, setup_deid | |||
| from .actions import perform_action | |||
| from pydicom.dataset import Dataset | |||
There was a problem hiding this comment.
This function should live with the others to load the deid, not with general utils.
| @@ -46,7 +46,7 @@ | |||
| import dateutil.parser | |||
There was a problem hiding this comment.
Looks like the DS STORE file snuck in? You can add this to the gitignore and remove the file or when you add files don't add entire folders at once.
deid/dicom/utils.py
Outdated
| @@ -280,3 +280,9 @@ def get_item_timestamp(dicom,date_field=None,time_field=None): | |||
|
|
|||
| return get_timestamp(item_date=item_date, | |||
| item_time=item_time) | |||
|
|
|||
| def setup_deid(deid): | |||
| # if the user has provided a custom deid, load it | |||
There was a problem hiding this comment.
Please add a documentation snippet to the function, and do y think we should support loading a base (default does not) and optionally taking a list of then to combine? Is setup the most appropriate descriptor for the function ?
There was a problem hiding this comment.
@vsoch I couldn't come up with a better name, but I don't think it's the most appropriate. I also don't like passing deid around with multiple meanings seems confusing to me when it sometimes refers to a file path and other times refers to the object.
There was a problem hiding this comment.
I completely agree - let me give a first shot at making a class and I'll ping you for review. A class would be important to avoid needing to re-generate the final thing, and also to (eventually) plug into generators and editors. Back in a bit!
|
@vsoch I think that's a good idea, but likely won't be able to get to it until after the break. |
|
haha no worries, I'll have something waiting for you when you get back then :) I also need to fix up the docs for the cleaner (that you couldn't find before) because I didn't properly write it down. Have a great break!! 🎅 🎄 ❄️ ⛄ 🕯️ |
|
okay the first dummy implementation is ready --> #37 It's likely not perfect, but when you are back let me know your feedback, and if the script and docs I provided for basic testing are missing in any detail. |
|
Closing, not addressed or updated by creator. |
No description provided.