-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Lungs in ct data #24
Lungs in ct data #24
Conversation
…-pipeline-structure 31 improve comments in the new pipeline structure
- Updated pipeline to new format - Added workaround in KITS19 - Added default_factory to DatasetArgs in all pipelines (possibly python version issue in 3.11)
- Split adding labels and ids to separate steps - Moved dataset preparation to abstract function that is called only when source path exists - Enabled not-creating Masks folder if it is set to None (for classification-only datasets)
…ne-for-coronahack [Coronahack] Create pipeline for Coronahack Dataset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please merge master into your changes
config/dataset_masks_config.py
Outdated
@@ -10,4 +10,7 @@ | |||
"kidney": 1, | |||
"kidney_tumor": 2, | |||
}, | |||
"Finding_and_Measuring_Lungs_in_CT_Data": { | |||
"lungs": 255, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is a problem, but 255 is already used by BrainMET
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok because it only refers to source images.
config/dataset_masks_config.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not using yamls anymore
config/dataset_uid_config.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Above
config/phases_config.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Above
config/runner_config.py
Outdated
# KITS21Pipeline( | ||
# path_args={ | ||
# "source_path": "", | ||
# "target_path": "./data/", | ||
# "labels_path": "", | ||
# }, | ||
# ), | ||
# StanfordCOCAPipeline( | ||
# path_args={ | ||
# "source_path": "", | ||
# "target_path": "./data/", | ||
# "masks_path": "", | ||
# }, | ||
# ), | ||
# StanfordBrainMETPipeline( | ||
# path_args={ | ||
# "source_path": "", | ||
# "target_path": "./data/", | ||
# }, | ||
# ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be commented
src/pipelines/base_pipeline.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simillar changes already in main. Please merge with main
image_folder_name="Images", | ||
mask_folder_name="Masks", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to mention it, then defaults will be used
src/steps/convert_tif2png.py
Outdated
# root_path = os.path.join(os.path.dirname(os.path.dirname(X[0])), self.mask_folder_name) | ||
# mask_paths = glob.glob(f"{root_path}/**/*.tif", recursive=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused commented code
src/steps/convert_tif2png.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try to refactor step so it is compatible with changes in #34
src/steps/convert_tif2png.py
Outdated
if self.mask_folder_name not in img_path: | ||
|
||
phase_id = self.phase_extractor(img_path) | ||
img_id = self.img_id_extractor(img_path) | ||
study_id = self.study_id_extractor(img_path) | ||
if phase_id not in self.phases.keys(): | ||
return None | ||
phase_name = self.phases[phase_id] | ||
new_file_name = f"{self.dataset_uid}_{phase_id}_{study_id}_{img_id}" | ||
new_file_name = new_file_name.replace(".tif", ".tiff") | ||
# Changing .tif to .tiff, so images will be readable for PIL | ||
tiff_path = os.path.join( | ||
self.target_path, | ||
f"{self.dataset_uid}_{self.dataset_name}", | ||
phase_name, | ||
self.image_folder_name, | ||
new_file_name, | ||
) | ||
shutil.copy2(img_path, tiff_path) | ||
new_path = tiff_path.replace(".tiff", ".png") | ||
try: | ||
image = PIL.Image.open(img_path) | ||
invert = True if np.min(np.array(image)) < 0 else False | ||
image = image.convert("L") | ||
if invert: | ||
image = PIL.ImageOps.invert(image) | ||
image.save(new_path, format="png") | ||
os.remove(tiff_path) | ||
except IOError: | ||
print("Image not found") | ||
else: | ||
new_path = img_path.replace(".tif", ".png").replace(".tiff", ".png") | ||
try: | ||
# Change .tif to .tiff, so images will be readable for PIL | ||
tiff_path = img_path.replace(".tif", ".tiff") | ||
os.rename(img_path, tiff_path) | ||
img_path = tiff_path | ||
image = PIL.Image.open(img_path) | ||
invert = True if np.min(np.array(image)) < 0 else False | ||
image = image.convert("L") | ||
if invert: | ||
image = PIL.ImageOps.invert(image) | ||
image.save(new_path, format="png") | ||
os.remove(img_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some comments. Describe what are the differences between what happens in if statement, and else statement.
The code in lines 116-124, and 132-140 looks identical to me, why do we need to duplicate it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, changed
# Conflicts: # config/dataset_masks_config.py # config/runner_config.py # poetry.lock # src/pipelines/base_pipeline.py
|
There was weird bug with mypy and precommit, so I added one one line to precommit config based on this thread
python/mypy#10632
Please check it.
Pipeline works ok.