454 refactored Encoding/Decoding logic to expose DICOM encoding mappings for consumption
Fixes #454.
Although, I did not find any bugs with dcmjs, some of the tooling in the library could be better exposed for consumption in other projects. I focus on exposing the DICOM buffer decoding here given an initial SpecificCharacterSet. If translation is not necessary, a few convenience methods are provided to set the correct encoder or decoder on demand.
This is a code cleanup / reorganization kind of PR.
Jest tests passed. I did not add any new tests since there is coverage from the current tests and the one class I added is trivial.
Deploy Preview for dcmjs2 ready!
| Name | Link |
|---|---|
| Latest commit | 50327d2ded87238d900c9593b88bdd5824483344 |
| Latest deploy log | https://app.netlify.com/projects/dcmjs2/deploys/68a4873833dd5100087c47e8 |
| Deploy Preview | https://deploy-preview-455--dcmjs2.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify project configuration.
@pieper I went ahead and did some cleanup based on your comments in #454. In terms of the other issue (#373), I do not do anything here other than change the location of the logic in question which I partially move lower in the abstraction hierarchy. However, if I get some time in the near future and no one has addressed it, I might take a stab. I think that the reasonable approach is to find the specific character set at the level of the tree in the header and use that for that level only. That can get a bit complex than I have time at the moment and I will need to consult the standard for other potential cases.
From a quick look I think the PR looks good 👍
I'd like another set of eyes on it though, so hopefully @wayfarer3130 can take a look too.
@wayfarer3130 checking in if there is anything you think I should improve on in this PR. No rush, I know you have been very helpful in other PRs and very busy. I am just helping keep this PR in people's minds in case there is a chance to merge. Thank you!