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

Add hazenlib init tests coverage #144

Merged
merged 28 commits into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9c66202
creating test functions for hazenlib.__init__
Lucrezia-Cester Nov 9, 2021
fd1bea4
added test functions to __hazenlib.init
Lucrezia-Cester Nov 15, 2021
98241ce
finished adding tests for functions in hazenlib
Lucrezia-Cester Nov 17, 2021
d3a3217
Update __init__.py
Lucrezia-Cester Nov 18, 2021
bf4f53e
added test for get_field_of_view
Lucrezia-Cester Nov 19, 2021
507526c
Merge branch 'develop' into add-hazelib.__init__-tests-coverage
Lucrezia-Cester Nov 22, 2021
0ee77f6
Update __init__.py
Lucrezia-Cester Nov 22, 2021
6b5491b
added tests for hazenlib.main()
Lucrezia-Cester Nov 24, 2021
bc42917
updated hazenlib.__main__
Lucrezia-Cester Nov 24, 2021
d6137d8
fixed failing tests
Lucrezia-Cester Nov 24, 2021
d2d224e
updated hazenlib_tests
Lucrezia-Cester Nov 24, 2021
008df9e
updated hazenlib_tests
Lucrezia-Cester Nov 24, 2021
39353b7
re added is_dicom_file
Lucrezia-Cester Nov 29, 2021
4afff59
put import statements at the beginning of the file
Lucrezia-Cester Nov 29, 2021
1ee8679
added *.logs to gitignore
Lucrezia-Cester Nov 30, 2021
54e6fb1
updated hazenlib_init_ tests
Lucrezia-Cester Dec 8, 2021
858c2f2
Merge branch 'release/0.5.0' into add-hazelib.__init__-tests-coverage
Lucrezia-Cester Dec 9, 2021
238be87
Update __init__.py
Lucrezia-Cester Dec 9, 2021
f9cc8c4
changed *.logs to *.log
Lucrezia-Cester Dec 22, 2021
44c2ddc
deleted logs file
Lucrezia-Cester Dec 22, 2021
2510d13
split big function into single functions
Lucrezia-Cester Dec 22, 2021
aef3f7a
Merge branch 'release/0.5.2' into add-hazelib.__init__-tests-coverage
Lucrezia-Cester Dec 22, 2021
51fa679
inserted pprint before return
Lucrezia-Cester Dec 22, 2021
658eab3
Merge branch 'release/0.5.2' into add-hazelib.__init__-tests-coverage
Lucrezia-Cester Dec 22, 2021
ada0244
updated init.py
Lucrezia-Cester Dec 22, 2021
0c05753
updates tests to work with pprint strings
Dec 22, 2021
fdbecc9
almsot equal relaxometry tests and set -Eeuxo pipefail
Dec 22, 2021
28f4690
adds entry point to allow us to return a string without exit code 0
Dec 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Hazen_logger.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
2021-11-15 21:19:24 I [spatial_resolution.py:main:514] 'FileDataset' object has no attribute 'SeriesDescription'
2021-11-15 21:20:56 I [spatial_resolution.py:main:514] 'FileDataset' object has no attribute 'SeriesDescription'
2021-11-15 21:28:24 I [spatial_resolution.py:main:514] 'FileDataset' object has no attribute 'SeriesDescription'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add all *.log files to the .gitignore so we don't track them?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, can you also delete this log from the repo?

52 changes: 31 additions & 21 deletions hazenlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,10 @@
import pydicom
from docopt import docopt
import numpy as np
import cv2
from hazenlib.tools import is_dicom_file


__version__ = '0.3.0'

import hazenlib.exceptions
Expand All @@ -106,10 +108,8 @@ def rescale_to_byte(array):
image_histogram, bins = np.histogram(array.flatten(), 255)
cdf = image_histogram.cumsum() # cumulative distribution function
cdf = 255 * cdf / cdf[-1] # normalize

# use linear interpolation of cdf to find new pixel values
image_equalized = np.interp(array.flatten(), bins[:-1], cdf)

return image_equalized.reshape(array.shape).astype('uint8')


Expand Down Expand Up @@ -310,13 +310,33 @@ def get_field_of_view(dcm: pydicom.Dataset):



def parse_relaxometry_data(task, arguments, dicom_objects, report): #def parse_relaxometry_data(arguments, dicom_objects, report): #

# Relaxometry arguments
relaxometry_cli_args = {'--calc_t1', '--calc_t2', '--plate_number',
'--show_template_fit', '--show_relax_fits',
'--show_rois', '--verbose'}

# Pass arguments with dictionary, stripping initial double dash ('--')
relaxometry_args = {}

for key in relaxometry_cli_args:
relaxometry_args[key[2:]] = arguments[key]

return task.main(dicom_objects, report_path = report,
**relaxometry_args)






def main():
arguments = docopt(__doc__, version=__version__)

task = importlib.import_module(f"hazenlib.{arguments['<task>']}")
folder = arguments['<folder>']
files = [os.path.join(folder, x) for x in os.listdir(folder) if x not in EXCLUDED_FILES]
dicom_objects = [pydicom.read_file(x, force=True) for x in files if is_dicom_file(x)]
dicom_objects = [pydicom.read_file(x, force=True) for x in files]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you re-add your if is_dicom_file condition here? It looks like this branch was forked before the is_dicom_file check was added. We need to add this back in here or it will overwrite this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am giving thumbs up to comments I have acted upon

pp = pprint.PrettyPrinter(indent=4, depth=1, width=1)
if arguments['--report']:
report = True
Expand All @@ -332,34 +352,24 @@ def main():

}


if arguments['--log'] in log_levels.keys():
level = log_levels[arguments['--log']]
logging.getLogger().setLevel(level)
else:
# logging.basicConfig()
logging.getLogger().setLevel(logging.INFO)

# logging.basicConfig()
logging.getLogger().setLevel(logging.INFO)


if not arguments['<task>'] == 'snr' and arguments['--measured_slice_width']:
raise Exception("the (--measured_slice_width) option can only be used with snr")
elif arguments['<task>'] == 'snr' and arguments['--measured_slice_width']:
measured_slice_width = float(arguments['--measured_slice_width'])
return pp.pprint(task.main(dicom_objects, measured_slice_width, report_path=report))
return task.main(dicom_objects, measured_slice_width, report_path=report)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why have you chosen to return the output outside of a pprint object? How does this effect the output the user sees?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not done to change the output the user sees, rather for me to be able to test the function because print returns NONE

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean print returns none or pp.pprint returns none? Could you add some screenshots to example outputs to explain what you mean? I don't think we want to change this if we can.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Lucrezia-Cester, would that mean that the user does not see the result printed to the command line when they run the SNR task? Could you try something like the below so we print the result and we also return a value?

    if not arguments['<task>'] == 'snr' and arguments['--measured_slice_width']:
        raise Exception("the (--measured_slice_width) option can only be used with snr")
    elif arguments['<task>'] == 'snr' and arguments['--measured_slice_width']:
        measured_slice_width = float(arguments['--measured_slice_width'])
        result = task.main(dicom_objects, measured_slice_width, report_path=report)
    elif arguments['<task>'] == 'relaxometry':
        result = parse_relaxometry_data(task, arguments, dicom_objects, report)
    else:
        result = task.main(dicom_objects, report_path=report)

    pp.pprint(result)

    return result


if arguments['<task>'] == 'relaxometry':
# Relaxometry arguments
relaxometry_cli_args = {'--calc_t1', '--calc_t2', '--plate_number',
'--show_template_fit', '--show_relax_fits',
'--show_rois', '--verbose'}

# Pass arguments with dictionary, stripping initial double dash ('--')
relaxometry_args = {}
return parse_relaxometry_data(task, arguments, dicom_objects, report)
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 we need to re-add this relaxometry code due to the earlier branching from develop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is still there, I have put this relaxometry code in a function outside the main function.


for key in relaxometry_cli_args:
relaxometry_args[key[2:]] = arguments[key]
return pp.pprint(task.main(dicom_objects, report_path=report))

return pp.pprint(task.main(dicom_objects, report_path=report,
**relaxometry_args))

return pp.pprint(task.main(dicom_objects, report_path=report))

2 changes: 2 additions & 0 deletions hazenlib/spatial_resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ def get_esf(edge_arr, y):
return u, esf




def calculate_mtf_for_edge(dicom, edge, report_path=False):
pixels = dicom.pixel_array
pe = dicom.InPlanePhaseEncodingDirection
Expand Down
Binary file added tests/data/resolution/philips/non_dicom_test.jfif
Binary file not shown.
194 changes: 192 additions & 2 deletions tests/test_hazenlib.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,45 @@
"""
Tests functions in the hazenlib.__init__.py file
"""


import unittest
import pydicom
import hazenlib
import os
import hazenlib.tools as hazen_tools
import numpy as np




from tests import TEST_DATA_DIR


class TestHazenlib(unittest.TestCase):
# Data by ImplementationVersionName
# all test values are taken from DICOM headers

MANUFACTURER = "philips"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add the file path here too? Then it is clearer that these parameters are the true values for that dicom. i.e. TEST_DICOM = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm') and replace the read_file method input with TEST_DICOM

ROWS = 512
COLUMNS = 512
TR_CHECK = 500
BW = 205.0
ENHANCED = False
PIX_ARRAY = 1





def setUp(self):
self.file = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm')
self.dcm = pydicom.read_file(self.file)

def test_get_bandwidth(self):
bw = hazenlib.get_bandwidth(self.dcm)
assert bw == self.BW



def test_get_rows(self):
rows = hazenlib.get_rows(self.dcm)
assert rows == self.ROWS
Expand All @@ -37,9 +52,47 @@ def test_get_TR(self):
TR = hazenlib.get_TR(self.dcm)
assert TR == self.TR_CHECK

def test_is_enhanced_dicom(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have a second test for an ENHANCED = True dicom too? just need to copy/paste this one but with self.dcm overwritten with an enhanced dicom and assert that returns True

enhanced = hazenlib.is_enhanced_dicom(self.dcm)
assert enhanced == self.ENHANCED

def test_get_num_of_frames(self):
pix_arr = hazenlib.get_num_of_frames(self.dcm)
assert pix_arr == self.PIX_ARRAY

def test_get_slice_thickness(self):
SLICE_THICK = self.dcm.SliceThickness
slice_thick = hazenlib.get_slice_thickness(self.dcm)
assert slice_thick == SLICE_THICK

def test_get_pixel_size(self):
PIX_SIZE = self.dcm.PixelSpacing
PIX_SIZE=tuple(PIX_SIZE)
pix_size = hazenlib.get_pixel_size(self.dcm)
assert pix_size == PIX_SIZE

def test_get_average(self):
AVG = self.dcm.NumberOfAverages
avg = hazenlib.get_average(self.dcm)
assert avg == AVG

def test_get_manufacturer(self):
assert hazenlib.get_manufacturer(self.dcm) == self.MANUFACTURER

file = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm')
dcm = pydicom.read_file(file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this bit so it inside a method of the class it's in? At the moment being outside means it will run every time the class is instantiated, this will also run before calling the setUp method, which is probably why we need to redefine the dcm and file variables.

FOV = dcm.Columns * dcm.PixelSpacing[0]
fov = hazenlib.get_field_of_view(dcm)
assert fov == FOV





class TestFactorsPhilipsMR531(TestHazenlib):
#PHILIPS_MR_53_1

MANUFACTURER = 'philips'
Copy link
Collaborator

Choose a reason for hiding this comment

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

you shouldn't need to define the test dicom values again here. They should be inherited from the first TestHazenLib class since this class inherits from TestHazenLib it should also inherit its properties.

ROWS = 512
COLUMNS = 512
TR_CHECK = 500
Expand All @@ -49,41 +102,178 @@ def setUp(self):
self.file = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm')
self.dcm = pydicom.read_file(self.file)

file = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm')
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, this bit looks like it needs to be ina function

dcm = pydicom.read_file(file)
FOV = dcm.Columns * dcm.PixelSpacing[0]
fov = hazenlib.get_field_of_view(dcm)
assert fov == FOV



class TestFactorsSiemensMRVE11C(TestHazenlib):
#SIEMENS_MR_VE11C
ROWS = 256
COLUMNS = 256
TR_CHECK = 500
BW = 130.0
MANUFACTURER = 'siemens'

def setUp(self):
self.file = str(TEST_DATA_DIR / 'resolution' / 'eastkent' / '256_sag.IMA')
self.dcm = pydicom.read_file(self.file)
FOV = self.dcm.Columns * self.dcm.PixelSpacing[0]

file = str(TEST_DATA_DIR / 'resolution' / 'eastkent' / '256_sag.IMA')
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.

dcm = pydicom.read_file(file)
FOV = dcm.Columns * dcm.PixelSpacing[0]
fov = hazenlib.get_field_of_view(dcm)
assert fov == FOV


class TestFactorsToshibaTMMRDCMV30(TestHazenlib):
# TOSHIBA_TM_MR_DCM_V3_0
ROWS = 256
COLUMNS = 256
TR_CHECK = 45.0
BW = 244.0
MANUFACTURER = 'toshiba'

def setUp(self):
self.file = str(TEST_DATA_DIR / 'toshiba' / 'TOSHIBA_TM_MR_DCM_V3_0.dcm')
self.dcm = pydicom.read_file(self.file)
FOV = self.dcm.Columns * self.dcm.PixelSpacing[0]


file = str(TEST_DATA_DIR / 'toshiba' / 'TOSHIBA_TM_MR_DCM_V3_0.dcm')
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

dcm = pydicom.read_file(file)
FOV = dcm.Columns * dcm.PixelSpacing[0]
fov = hazenlib.get_field_of_view(dcm)
assert fov == FOV




class TestFactorsGEeFilm(TestHazenlib):
# GE_eFILM
ROWS = 256
COLUMNS = 256
TR_CHECK = 1000.0
BW = 156.25
MANUFACTURER ='ge'

def setUp(self):
self.file = str(TEST_DATA_DIR / 'ge' / 'ge_eFilm.dcm')
self.dcm = pydicom.read_file(self.file)


file = str(TEST_DATA_DIR / 'ge' / 'ge_eFilm.dcm')
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

dcm = pydicom.read_file(file)
#FOV = dcm.Columns * dcm.PixelSpacing[0]
#fov = hazenlib.get_field_of_view(dcm)
#print(fov)





class Test(unittest.TestCase):

def setUp(self):
self.file = str(TEST_DATA_DIR / 'resolution' / 'eastkent' / '256_sag.IMA')
self.dcm = pydicom.read_file(self.file)
self.dcm = self.dcm.pixel_array

def test_isupper(self):
test_array = np.array([[1,2], [3,4]])
TEST_OUT = np.array([[63,127],[191,255]])
test_array = hazenlib.rescale_to_byte(test_array)
test_array = test_array.tolist()
TEST_OUT = TEST_OUT.tolist()
self.assertListEqual(test_array, TEST_OUT)



from docopt import docopt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you move all import statements so they're at the top of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @laurencejackson, thank you for all the feedback. I'll get to it in the following days


from hazenlib.logger import logger

from pprint import pprint

import sys

import os


class TestCliParser(unittest.TestCase):

def setUp(self):
self.file = str(TEST_DATA_DIR / 'resolution' / 'philips' / 'IM-0004-0002.dcm')
self.dcm = pydicom.read_file(self.file)


def test1_logger(self):
sys.argv = ["hazen", "spatial_resolution", ".\\tests\\data\\resolution\\RESOLUTION\\", "--log", "warning"]

sys.argv = [item.replace("\\","/") for item in sys.argv]

hazenlib.main()

logging = hazenlib.logging

self.assertEqual(logging.root.level, logging.WARNING)


def test2_logger(self):
sys.argv = ["hazen", "spatial_resolution", ".\\tests\\data\\resolution\\RESOLUTION\\"]

sys.argv = [item.replace("\\", "/") for item in sys.argv]

hazenlib.main()

logging = hazenlib.logging

self.assertEqual(logging.root.level, logging.INFO)


def test_main_snr_exception(self):
sys.argv = ["hazen", "spatial_resolution", ".\\tests\\data\\snr\\Siemens\\", "--measured_slice_width=10"]

sys.argv = [item.replace("\\", "/") for item in sys.argv]

self.assertRaises(Exception, hazenlib.main)

def test_snr_measured_slice_width(self):
sys.argv = ["hazen", "snr", ".\\tests\\data\\snr\\GE", "--measured_slice_width", "1"]

sys.argv = [item.replace("\\", "/") for item in sys.argv]

output=hazenlib.main()

dict1={'snr_subtraction_measured_SNR SAG MEAS1_23_1': 183.97,
'snr_subtraction_normalised_SNR SAG MEAS1_23_1': 7593.04,
'snr_smoothing_measured_SNR SAG MEAS1_23_1': 184.41,
'snr_smoothing_normalised_SNR SAG MEAS1_23_1': 7610.83,
'snr_smoothing_measured_SNR SAG MEAS2_24_1': 189.38,
'snr_smoothing_normalised_SNR SAG MEAS2_24_1': 7816.0}

self.assertDictEqual(dict1, output)


def test_relaxometyr(self):
sys.argv = ["hazen", "relaxometry", ".\\tests\\data\\relaxometry\\T1\\site3_ge\\plate4\\", "--plate_number", "4", "--calc_t1"]

sys.argv = [item.replace("\\", "/") for item in sys.argv]

output = hazenlib.main()

dict1 = {'Spin Echo_32_2_P4_t1': {'rms_frac_time_difference': 0.13499936644959426}}
self.assertDictEqual(dict1, output)

if __name__ == "__main__":
unittest.main()