-
Notifications
You must be signed in to change notification settings - Fork 373
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
Introduce DeserializableComponent
trait and high-level query_latest
#1417
Conversation
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.
pretty! seems rather straight forward to me :)
/// A [`Component`] that fulfils all the conditions required to be deserialized from an Arrow | ||
/// payload. | ||
/// | ||
/// Note that due to the use of HRTBs in `arrow2_convert` traits, you will still need an extra HRTB |
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.
HRTB
for the uninitiated?
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.
https://doc.rust-lang.org/nomicon/hrtb.html
An old-school workaround for the lack of generic associated types... which actually have landed since then, at least in part.
We should be able to make those APIs in arrow2_convert
friendlier pretty soon.
/// we check to see if it exists in the arrow storage. | ||
pub fn query_transform( | ||
/// Get the latest value for a given [`re_log_types::msg_bundle::Component`]. | ||
pub fn query_latest<C: DeserializableComponent>( |
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.
should there be something in the name or comment on how this expects that there is only a single component of the type on the entity?
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.
That's already guaranteed by the data model 🤔 Ha, you mean a single instance. Yes.
…t` (#1417) * introduce DeserializableComponent * turn query_transform into a generic query_latest * use DeserializableComponent everywhere it makes sense * use SerializableComponent everywhere it makes sense * self review * addressed PR comments
This adds a
DeserializableComponent
trait (which does what you think it does and means what you think it means) and uses it everywhere it makes sense.Also fixed a bunch of places where
SerializableComponent
wasn't used yet.In addition to this, this generalizes the
query_transform
function (that we use in a bunch of places in the UI to fetch the latest transform of a given entity path) to any component.We do so because I now have the need to fetch stuff other than transforms when displaying the selection panel.