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

Refactor read* methods into a lightweight reader struct. #254

Merged
merged 4 commits into from
Feb 20, 2023

Conversation

suyashkumar
Copy link
Owner

@suyashkumar suyashkumar commented Feb 19, 2023

This change wraps the read* methods in read.go into a lightweight reader struct. This allows for easier sharing of some common variables, for example the parse options set without having to inject them through deep read* call chains.

This also does the following:

  • Moves readMetadata into read.go
  • removes parseOptSet from Dataset.
  • updates dicomio.NewReader signature to not include an error (not needed).

We may want to revisit the naming of some of these entities at some point. There now exists a dicom.reader (unexported) and a lower level dicomio.Reader (referred to as rawReader in this change). Ideally we can also make future refactors to eliminate the need for Parser to be aware of the rawReader (should be easy).

@suyashkumar suyashkumar changed the title Prototype reader refactor. Struct naming not final. Refactor read* methods into a lightweight reader struct. Feb 20, 2023
@suyashkumar suyashkumar self-assigned this Feb 20, 2023
@suyashkumar suyashkumar marked this pull request as ready for review February 20, 2023 00:13
@suyashkumar suyashkumar merged commit a0d9656 into main Feb 20, 2023
suyashkumar added a commit that referenced this pull request Feb 20, 2023
This introduces an option to SkipPixelData processing, making use of the new reader struct introduced in #254.

See also, review discussion in #243 that helped motivate the changes I wanted to make in #254.
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.

1 participant