-
Notifications
You must be signed in to change notification settings - Fork 528
Provide AnyValues
helpers in Rust SDK
#10074
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
Conversation
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
d546580
to
b68f3c3
Compare
Arc::new(arrow::array::StringArray::from(vec![ | ||
"https://www.rerun.io", | ||
])), | ||
) | ||
.with_field( | ||
"repository", | ||
Arc::new(arrow::array::StringArray::from(vec![ | ||
"https://github.com/rerun-io/rerun", | ||
])), |
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.
If we use impl Into<ArrayRef>
as the argument type, maybe we can get rid of the Arc::new
here?
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.
I tried this, but unfortunately could not get the compiler to comply.
be8b72d
to
f860d0d
Compare
AnyValues
across all SDKs and use archetype_field_name
as discriminatorAnyValues
helpers in Rust SDK
b813e71
to
1fed3a5
Compare
/// Assigns an (archetype) name to this set of any values. | ||
#[inline] | ||
pub fn new(archetype_name: impl Into<ArchetypeName>) -> Self { |
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.
I think most users will first grab for AnyValues::new()
, and maybe not even realize there is a AnyValues::default()
, and so they will wonder what they should pick for archetype name.
So I suggest we do builder pattern all the way instead:
AnyValues::new().with_field(…)
// vs
AnyValues::new().with_archtype_name(…).with_field(…)
or alternatively use a more verbose ctor name:
AnyValues::new().with_field(…)
// vs
AnyValues::from_archtype_name(…).with_field(…)
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.
I deliberately made that choice to nudge users to provide an archetype name, which should be good practice going forward as it helps with uniquely identifying the data going forward.
Latest documentation preview deployed successfully.
Note: This comment is updated whenever you push a commit. |
Related
AnyValues
should use argument names to setarchetype_field_name
metadata #9906.AnyValues
wrapper to C++ API #9340.What
This PR does multiple things:
AnyValues
interface that is similar across Python 🐍 , C++ 🌊 , and Rust 🦀 SDKs (so far we only haveAnyValues
in Python).AnyValues
we ensure that more of the logged data is actually tagged.