-
Notifications
You must be signed in to change notification settings - Fork 84
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
ENH: Add functionality for mutating element values #471
Conversation
If this is something you'd like, I'll go ahead and polish by adding docs, tests, and cleaning up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this proposal. Unfortunately, some methods of that nature were deliberately kept away from the object API until now because it opens room for inconsistencies in the object. More details inline.
Removed or switched to private the methods you mentioned would be dangerous. Ended up keeping exposed
Also called out in the docs for each function the alternatives (i.e. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the reiteration. Just some finishing touches and it should be ready.
* Add `value_at_mut`, `entry_at`, `entry_at_mut` and `update_value_at` methods on `InMemDicomObject` struct. * Add supporting methods to dependent objects
* Remove dangerous methods * Add documentation for new methods that are staying
- remove duplicate value_at method
4ab3571
to
6978b2c
Compare
- needless borrow in `.get(&tag)` - work around rust-analyzer bug in line 1023
41636bc
to
7542ff5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, this is ready. Thank you @naterichman. 👍
value_at_mut
,entry_at
,entry_at_mut
andupdate_value_at
methods onInMemDicomObject
struct.Let me know if this is something you would like, I'm finding I need something like this for a downstream application. I realize I can use the
update_value
method, but its nice to be able toupdate_value
with anAttributeSelector
. If there is a way to do that and I'm missing it, I'm happy to just update the docs to make that clearer instead of this PR.