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

Changes to DicomValue API and sequences #353

Merged
merged 4 commits into from
Jun 5, 2023
Merged

Conversation

Enet4
Copy link
Owner

@Enet4 Enet4 commented Jun 4, 2023

This makes some usability improvements to the core value API. The main changes here are the enum DicomValue<I, P> now has a different default for the type parameter P (to the same value used by in-memory DICOM objects in dicom_object), and it uses struct definitions for data set sequences and pixel data fragment sequence, instead of declaring struct variants in the enum.

I also took this opportunity to remove deprecated functions, adjust the error types returned in some methods, and increase test coverage a bit.

Summary

  • Move DicomValue sequence variant fields to their own independent types (DataSetSequence and PixelFragmentSequence)
  • Change default of Value type parameter P to Vec<u8> (also aliased to InMemFragment)
  • Change the return error type of Value::to_str, Value::to_raw_str, Value::to_bytes
    • from CastValueError to ConvertValueError
  • Change behavior of PartialEq for DicomValue between data set items: recorded length is ignored, only items are compared.

@Enet4 Enet4 added breaking change Hint that this may require a major version bump on release C-core Crate: dicom-core labels Jun 4, 2023
Enet4 added 3 commits June 4, 2023 21:59
- (change) rename `ValueType::Item` to `DataSetSequence`
   - this name describes what it represents more accurately
- add `DataSetSequence` and `PixelFragmentSequence`
- (change) replace `Value` variants `Sequence` and `PixelSequence`
   - use a tuple struct to a specific sequence type instead
- Clarify that `new_sequence` and `new_pixel_sequence`
  only work in isolation
-  [parser][object][pixeldata][dump] propagate changes to other crates
- Vec<u8> is more common and more practical than [u8; 0]
- Add alias `InMemFragment`
- [object] redefine `InMemFragment` to use the definition from core
- change PartialEq impl for DataSetSequence
   - disregards recorded length
- affects to_str(), to_raw_str(), and to_bytes()
- ConvertValueError makes more sense here
- (change) remove deprecated methods to_clean_str()
- propagate error type change to data element types
- increase module test coverage
@Enet4 Enet4 force-pushed the change/core/value-seq branch from 58e00b4 to 5147830 Compare June 4, 2023 20:59
@Enet4 Enet4 merged commit b5f1224 into master Jun 5, 2023
@Enet4 Enet4 deleted the change/core/value-seq branch June 5, 2023 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Hint that this may require a major version bump on release C-core Crate: dicom-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant