Handle unknowns completely - Githubissues

suyashkumar / dicom

⚡High Performance DICOM Medical Image Parser in Go.

MIT License

933 stars 136 forks source link

Handle unknowns completely #235

Closed jabillings closed 2 months ago

jabillings commented 2 years ago

This PR is adapted from the previous one.

The main new addition it makes to this existing s/read-un-as-sq branch is treating unknown elements* as bytes not strings. It also contains changes from a main branch rebase.

*outside of the implicit syntax+undefined length case

suyashkumar commented 2 years ago

Oh hey, there's some build failures due to a duplicate switch/case as well.

jabillings commented 2 years ago

Huh, I'm just not seeing the duplicate switch/case. I only see the tag.VRUnkown type once in this switch case (and the entire file, for that matter).

suyashkumar commented 2 years ago

Hey! Going to close and re-open this to re-run the checks, and start looking into what's up with the build here.

suyashkumar commented 2 years ago

Seems like updating the go version helps with this? See https://github.com/suyashkumar/dicom/pull/242 If you merge https://github.com/jabillings/dicom/pull/1 into your branch that should help!

suyashkumar commented 2 years ago

For some reasons the checks are not triggering--looking into this more.

suyashkumar commented 2 years ago

Could be the issue that benchstat isn't found in the GitHub actions environment for some reason. Maybe they've changed the way they handle paths with respect to GOBIN. Going to see if fixing that does anything.

suyashkumar commented 2 years ago

Also, I see you're wanting to merge this into s/read-un-as-sq--that seems reasonable, I can merge this into there and then polish it up as needed before merging into main (I'll list you as a co-author of course). I'll take it from here! Thank you for the contribution!

atreyamj commented 9 months ago

Hi Folks, I am new to the community, first off - thank you for all the contributions - I was wondering if this PR was already merged to main and if the repo lib handles UN tags?

Thank You!