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

Old DICOM file loads using dcm_parse(file; preamble=false), but the same file returns false when runningisdicom function #84

Open
Dale-Black opened this issue Feb 16, 2022 · 4 comments

Comments

@Dale-Black
Copy link
Contributor

I am trying to load a directory of DICOM files using dcmdir_parse. Each file in the directory loads when running dcm_parse(file; preamble=false) but loading the directory returns an empty array. While trying to determine the cause, I found that isdicom(file) returns false, but dcm_parse(file; preamble=false) works. Any idea if this is an issue or if I am missing something on my end?

image

@notZaki
Copy link
Member

notZaki commented Feb 16, 2022

That is kinda the expected, albeit annoying, behaviour of isdicom() because it uses the preamble + prefix to identify dicom files as that's what the dicom file format suggests [ref].

On the package side, we could update isdicom() so that it also tries to read the first couple of bytes and if they look like valid dicom, then to assume that the rest of the file is dicom.
Either that, or make it skip isdicom() if preamble=false is used.

@Dale-Black
Copy link
Contributor Author

Dale-Black commented Feb 16, 2022

What about modifying find_dicom_files to not even need isdicom()? This is a super hacky example but seems to work at least for my use case

function find_dicom_files(dir; kwargs...)
    files = joinpath.(dir, readdir(dir))
	dicom_files = []
	for file in files
		try
			dcm = dcm_parse(file; kwargs...)
			push!(dicom_files, file)
		catch
			nothing
		end
	end
    return dicom_files
end

@Dale-Black
Copy link
Contributor Author

This would work for dcmdir_parse, at least in my use case

image

@notZaki
Copy link
Member

notZaki commented Feb 16, 2022

Since that function is already calling dcm_parse, you could make the following change to return the parsed data (maybe change the function name to reflect this):

function find_dicom_files(dir; kwargs...)
    files = joinpath.(dir, readdir(dir))
-	dicom_files = []
+	dcms = [] 
	for file in files
		try
			dcm = dcm_parse(file; kwargs...)
-			push!(dicom_files, file)
+			push!(dcms, dcm)
		catch
			nothing
		end
	end
-    return dicom_files
+    return dcms
end

or if everything in the folder is a dicom file, then the following could work too:

dir = "PATH/TO/DIR"
dcms = [DICOM.dcm_parse(file; preamble=false) for file in readdir(dir; join=true)]

The try/catch is a good practical solution, however, I think it would be better for the package if isdicom() could recognize files with missing preambles rather than changing the find_dicom_files directly.

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

No branches or pull requests

2 participants