Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lungs in ct data #24

Closed
wants to merge 131 commits into from
Closed

Lungs in ct data #24

wants to merge 131 commits into from

Conversation

Piotr1219
Copy link
Collaborator

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.

A-Huli and others added 16 commits December 6, 2023 17:58
…-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
Copy link
Collaborator

@andrekomor andrekomor left a 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

@@ -10,4 +10,7 @@
"kidney": 1,
"kidney_tumor": 2,
},
"Finding_and_Measuring_Lungs_in_CT_Data": {
"lungs": 255,
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

Copy link
Collaborator

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Above

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Above

Comment on lines 19 to 38
# 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/",
# },
# ),
Copy link
Collaborator

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

Copy link
Collaborator

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

Comment on lines 45 to 46
image_folder_name="Images",
mask_folder_name="Masks",
Copy link
Collaborator

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

Comment on lines 73 to 74
# 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused commented code

Copy link
Collaborator

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

Comment on lines 95 to 138
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)
Copy link
Collaborator

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?

Copy link
Collaborator Author

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
@Piotr1219
Copy link
Collaborator Author

  • Added FindingAndMeasuringLungsInCTPipeline()
  • Removed default values for image_folder_name and masks_folder_name in used steps
  • Removed 'dcm' from 'img_dcm_prefix' and 'segmentation_dcm_prefix'
  • Renamed copy_png_masks to copy_masks
  • Added tif to png conversion step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants