Add ComponentDescriptorRef to re_sorbet
#9043
Conversation
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
abey79
added
exclude from changelog
| impl ColumnDescriptorRef<'_> { | ||
| #[inline] | ||
| pub fn entity_path(&self) -> Option<&EntityPath> { | ||
| match self { | ||
| Self::RowId(_) | Self::Time(_) => None, | ||
| Self::Component(descr) => Some(&descr.entity_path), | ||
| } | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn short_name(&self) -> String { | ||
| match self { | ||
| Self::RowId(descr) => descr.name().to_owned(), | ||
| Self::Time(descr) => descr.timeline.name().to_string(), | ||
| Self::Component(descr) => descr.component_name.short_name().to_owned(), | ||
| } | ||
| } | ||
|
|
||
| #[inline] | ||
| pub fn is_static(&self) -> bool { |
There was a problem hiding this comment.
I know the existing ColumnDescriptor already had these functions, and that they are undocumented. But do we really need them? I prefer to remove them, as I find it hard to explain why the row-id column is not static, for instance.
If we do want them, then let us add docstrings for why their indended use is
There was a problem hiding this comment.
I'll remove all the ones I dont need (which is all but short_name)
| @@ -34,6 +34,12 @@ impl RowIdColumnDescriptor { | |||
| Self {} | |||
| } | |||
|
|
|||
| #[inline] | |||
| #[expect(clippy::unused_self)] | |||
| pub fn name(&self) -> &'static str { | |||
There was a problem hiding this comment.
What is the intentional use for this? If this is supposed to be the default column name, then I think we should use either PascalCase or snake_case. If this is supposed to be a human readable string, then I think "Row ID" is better.
There was a problem hiding this comment.
Human readable name, so "Row ID" it is.
Related
RowIdColumnDescriptortore_sorbet::ColumnDescriptor#9034What
This PR introduce the
ComponentDescriptorReftype tore_sorbet. It's likeComponentDescriptorbut:RowIdenum (which would be difficult to add toComponentDescriptorbecause of the ramifications on the dataframe API—let's wait for the RowID <-> Index column merge for that).