Skip to content
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

Create Rust version of the "Loggable Data Types" doc section #2501

Merged
merged 14 commits into from
Jun 27, 2023

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Jun 21, 2023

What

This PR introduces Rust versions of the "Loggable Data Types" documentation section.

Choices I made along the way:

  • The primary aim was to stick as much as possible to the Python versions. The output should be the same.
  • I've used ndarray for all example (as opposed to, e.g., ImageBuffer for image-related examples).
  • I skipped translating image_advanced.py as it primarily focuses various Python packages (Pillow, OpenCV).

Checklist

PR Build Summary: https://build.rerun.io/pr/2501

Docs preview: https://rerun.io/preview/72885f5/docs
Examples preview: https://rerun.io/preview/72885f5/examples

@abey79 abey79 added the 📖 documentation Improvements or additions to documentation label Jun 21, 2023
@abey79
Copy link
Member Author

abey79 commented Jun 23, 2023

These examples were exceedingly interesting to write, and are IMHO very useful to identify the shortcomings of the current Rust API. In particular, I ran into the following gotcha, many of which required @teh-cmc's help to sort out:

  • RawMesh3D.vertex_colors want packed RGBA u32, but, porting from Python, I first provided a flattened array of u8 RGB components (felt natural since the other fields also take flattened arrays of coordinates/normals/etc.). This would happily compile, but nothing was displayed and no error was presented.
  • When implementing the step timeline of the scalar example, I first searched for set_time_sequence (used in Python API) in the Rust docs, which I found in RecordingStream. Turns out it's not in 0.7 and bugged on main (or rather ignored by MsgSender). Again, it compiled and displayed no error, but no timeline was created.
  • There is a redundancy between the ClassID key in AnnotationContext.class_map and the ClassDescription.info.id field. It's unclear if they much match, or what's the meaning of each of them. I always matched them and didn't run into any issue, but it's still confusing.
  • Likewise for ClassDescription.keypoint_map: the hashmap's KeypointID is redundant with AnnotationInfo.id field and generates confusion.
  • Btw, AnnotationInfo.id is u16 but documented as "either ClassID or KeypointID". How un-rusty! :)
  • Still on the topic of annotation context, specifically for the "connection" example. To work, the Rust version requires a ClassId(0) splat to be logged with the points, presumably to match the AnnotationContext.class_map hashmap key. Inferring that was possible thanks to classid being listed as secondary component of Point2D/3D, but it was confusing that there appears to be no such equivalent (or hidden default) in the Python API.

@abey79 abey79 marked this pull request as ready for review June 23, 2023 07:56
@teh-cmc teh-cmc self-requested a review June 26, 2023 07:19
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, cannot wait to make them less painful 😄

docs/code-examples/Cargo.toml Outdated Show resolved Hide resolved
docs/code-examples/Cargo.toml Outdated Show resolved Hide resolved
docs/code-examples/mesh_simple.rs Outdated Show resolved Hide resolved
docs/code-examples/point2d_random.rs Show resolved Hide resolved
docs/code-examples/point3d_simple.rs Outdated Show resolved Hide resolved
docs/code-examples/scalar_simple.rs Outdated Show resolved Hide resolved
docs/code-examples/text_entry_simple.rs Outdated Show resolved Hide resolved
abey79 and others added 9 commits June 26, 2023 10:30
Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
teh-cmc added a commit that referenced this pull request Jun 26, 2023
(Probably easier to review commit by commit)

Follow up to my discussion with @abey79 regarding his poor experience
with time tracking, which was summarized in
#2501 (comment):
> - When implementing the `step` timeline of the scalar example, I first
searched for `set_time_sequence` (used in Python API) in the Rust docs,
which I found in `RecordingStream`. Turns out it's not in 0.7 and bugged
on `main` (or rather ignored by `MsgSender`). Again, it compiled and
displayed no error, but no timeline was created.

This PR makes it so that `RecordingStream` is always in charge of
injecting its internal clock into outgoing rows (unless the caller ask
it not to, e.g. because the data is meant to be timeless).
This is pretty similar to what was already in place for `log_tick`,
except it now applies to every timelines, whether they are builtin or
user defined.

- Within the Python SDK, this gets rid of all the existing manual time
injection stuff.
- On the Rust SDK's side, this fixes the issue that `MsgSender` used to
ignore the internal clock altogether (i.e. the stateful time APIs were
not supported at all for Rust users).
- And finally this cleans up the Rust examples a bunch since we now have
access to stateful time.


---

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2506

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/178edf5/docs
Examples preview: https://rerun.io/preview/178edf5/examples
<!-- pr-link-docs:end -->
@abey79 abey79 merged commit b6fdc00 into main Jun 27, 2023
@abey79 abey79 deleted the antoine/rust-examples branch June 27, 2023 08:14
emilk pushed a commit that referenced this pull request Jun 29, 2023
(Probably easier to review commit by commit)

Follow up to my discussion with @abey79 regarding his poor experience
with time tracking, which was summarized in
#2501 (comment):
> - When implementing the `step` timeline of the scalar example, I first
searched for `set_time_sequence` (used in Python API) in the Rust docs,
which I found in `RecordingStream`. Turns out it's not in 0.7 and bugged
on `main` (or rather ignored by `MsgSender`). Again, it compiled and
displayed no error, but no timeline was created.

This PR makes it so that `RecordingStream` is always in charge of
injecting its internal clock into outgoing rows (unless the caller ask
it not to, e.g. because the data is meant to be timeless).
This is pretty similar to what was already in place for `log_tick`,
except it now applies to every timelines, whether they are builtin or
user defined.

- Within the Python SDK, this gets rid of all the existing manual time
injection stuff.
- On the Rust SDK's side, this fixes the issue that `MsgSender` used to
ignore the internal clock altogether (i.e. the stateful time APIs were
not supported at all for Rust users).
- And finally this cleans up the Rust examples a bunch since we now have
access to stateful time.


---

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2506

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/178edf5/docs
Examples preview: https://rerun.io/preview/178edf5/examples
<!-- pr-link-docs:end -->
emilk pushed a commit that referenced this pull request Jun 29, 2023
### What

This PR introduces Rust versions of the "Loggable Data Types"
documentation section.

Choices I made along the way:
- The primary aim was to stick as much as possible to the Python
versions. The output should be the same.
- I've used `ndarray` for all example (as opposed to, e.g.,
`ImageBuffer` for image-related examples).
- I skipped translating `image_advanced.py` as it primarily focuses
various Python packages (Pillow, OpenCV).

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [ ] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2501

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/72885f5/docs
Examples preview: https://rerun.io/preview/72885f5/examples
<!-- pr-link-docs:end -->

---------

Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants