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

Export writeElement and readElement #206

Closed
wants to merge 3 commits into from
Closed

Export writeElement and readElement #206

wants to merge 3 commits into from

Conversation

kristianvalind
Copy link
Contributor

@kristianvalind kristianvalind commented Jun 21, 2021

I'm working on a port of the grailbio/go-netdicom package to use your maintained and more idiomatic implementation instead of grailbio/go-dicom. To be able to encode and decode DIMSE messages, I need to use per-element writing and reading. Rather than reimplementing this functionality myself, I propose exporting the readElement and writeElement functions so that go-netdicom or other packages that require low-level DICOM functionality may benefit.

@kristianvalind kristianvalind changed the title Export writeElement as WriteElement Export writeElement and readElement Jun 21, 2021
@suyashkumar
Copy link
Owner

Thanks @kristianvalind! Question for you--would it not be possible to use the Parser.Next() pattern to do element-by-element reading?

Would a similar Writer API help on the writing side?

Just curious--any more context on how you're approaching this would be helpful!

On my side, I don't think this is a huge deal, but also, I'm careful to export more ways to read an element because I'd like there to be one (or maybe two) canonical exported ways to do something (like read or write) in this library.

@kristianvalind
Copy link
Contributor Author

kristianvalind commented Jun 28, 2021

DIMSE messages consist of a specific set of DICOM elements, and the functionality I need is single element reading and writing from an already open Reader/Writer, so Parser.Next() seems like a good fit on the reading side. I had just not thought about it. Thanks! For writing, a Writer type with a similar API would probably be a cleaner solution than just exporting writeElement.

What would be the best way to implement such an API? I think it would be relatively easy to create a new Writer struct type with methods calling existing non-exported writing functions, but that would not follow the pattern of the Parser type, and might not be the most elegant way...

@kristianvalind
Copy link
Contributor Author

kristianvalind commented Jun 30, 2021

I'm closing this pull request and will submit another with a Writer struct implementation proposal.

@estergoldberg
Copy link

Hi @kristianvalind - do you have a netdicom project that use @suyashkumar's go-dicom package?
Did you really implement code for dicom query that using latest go-dicom package?
I need to implement 'Modality worklist' and also want to use netdicom and go-dicom packages
Thanks in advanced.

@kristianvalind
Copy link
Contributor Author

@estergoldberg Unfortunately, no.

I started a port of grailbio/go-netdicom to use @suyashkumar's go-dicom, but I haven't touched it in almost 2 years. If you want to have a look, some incomplete feature branches are available.

My goal was something that would be useful for MW, but I ended up calling findscu from dcmtk via os/exec instead.

@estergoldberg
Copy link

Thanks anyway, will take a look on your branches.

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.

3 participants