-
Notifications
You must be signed in to change notification settings - Fork 82
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
Revise DicomObject abstraction #524
Draft
Enet4
wants to merge
10
commits into
master
Choose a base branch
from
change/object/dicom-object-revision-gats
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+1,120
−80
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enet4
added
enhancement
A-lib
Area: library
C-object
Crate: dicom-object
breaking change
Hint that this may require a major version bump on release
labels
Jun 23, 2024
Enet4
force-pushed
the
change/object/dicom-object-revision-gats
branch
2 times, most recently
from
June 29, 2024 17:25
5236b94
to
c757898
Compare
- use GAT on Element type - add element_opt and element_by_name_opt
- 1-level deep copy of a DICOM value
- add DicomAttributeValue trait, use it as output of most methods in DicomObject - rename methods and add new ones in DicomObject - use GATs for the attribute value, item and pixel data types - reimplement DicomObject
- provide access to items and pixel data fragments - require DicomAttributeValue to impl DicomValueType - add a few tests
- rename from DicomAttributeValue - add `to_i32` and `to_u16`
- instead, meta attributes can be retrieved like any other attribute
- replace `to_dicom_value` with `to_primitive_value` - simplifies implementations and requires consumers to depend on other methods when working with sequences - remove method `meta` - treat meta information attributes like any other attribute, retrievable through the same methods - [core] add either crate - impl many DICOM traits to `either::Either` - implement DicomObject for FileMetaTable - reimplement DICOM traits for FileDicomObject so that users can retrieve either meta info or main data set info
- elide lifetime - fix formatting of doc comments
Enet4
force-pushed
the
change/object/dicom-object-revision-gats
branch
from
November 2, 2024 12:16
eae2801
to
e044a47
Compare
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-lib
Area: library
breaking change
Hint that this may require a major version bump on release
C-object
Crate: dicom-object
enhancement
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
A new iteration of the trait-based DICOM object abstraction, which may help uniformize DICOM object handling regardless of the underlying implementation.
Summary
Known caveats
get
are nice and short, but they conflict withInMemDicomObject::get
and do not work the same way. I might need to rename them again.