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

SDK DataLoaders 8: customizable (external) loaders for C++ #5361

Merged
merged 5 commits into from Mar 4, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Feb 29, 2024

Introduces the new DataLoaderSettings business to C++ and update examples accordingly (external_data_loader & log_file).

./build/debug/examples/cpp/log_file/example_log_file --recording-id this-one --entity-path-prefix a/b/c  --time sim_time=1000 --time wall_time=1709204046 --sequence sim_frame=42 rerun_cpp/tests/main.cpp | rerun -

image

Checks:

  • external loader ran manually (loader.exe | rerun)
  • external loader via rerun (rerun xxx.cpp)
  • log_file with external loader (log_file xxx.cpp)

Part of series of PR to expose configurable DataLoaders to our SDKs:

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

@teh-cmc teh-cmc added do-not-merge Do not merge this PR 🌊 C++ API C or C++ logging API include in changelog 🪵 Log-API Affects the user-facing API for all languages labels Feb 29, 2024
@teh-cmc teh-cmc marked this pull request as draft February 29, 2024 17:53
Base automatically changed from cmc/sdk_dataloader_6_python_settings to main March 1, 2024 16:31
teh-cmc added a commit that referenced this pull request Mar 1, 2024
Introduces the new `DataLoaderSettings` business to Python and update
examples accordingly (`external_data_loader` & `log_file`).

```bash
python examples/python/external_data_loader/main.py --recording-id this-one --entity-path-prefix a/b/c  --time sim_time=1000 --time wall_time=1709204046 --sequence sim_frame=42 examples/python/dna/main.py | rerun -
```

![image](https://github.com/rerun-io/rerun/assets/2910679/bfda567d-3d16-42cd-be8e-8b1a0767a784)



Checks:
- [x] external loader ran manually (`python loader | rerun`)
- [x] external loader via rerun (`rerun xxx.py`)
- [x] log_file with external loader (`log_file xxx.py`)

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5361
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_7_cpp_settings branch 3 times, most recently from f77e8b4 to cc02362 Compare March 4, 2024 10:32
@teh-cmc teh-cmc changed the title SDK DataLoaders 7: customizable (external) loaders for C++ SDK DataLoaders 8: customizable (external) loaders for C++ Mar 4, 2024
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_7_cpp_settings branch from cc02362 to 76d84d8 Compare March 4, 2024 11:31
@teh-cmc teh-cmc changed the base branch from main to cmc/sdk_dataloader_7_stateful_log_file March 4, 2024 11:32
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_7_cpp_settings branch 2 times, most recently from 7ae4317 to dcdfca3 Compare March 4, 2024 14:52
@teh-cmc teh-cmc marked this pull request as ready for review March 4, 2024 14:54
@teh-cmc teh-cmc mentioned this pull request Mar 4, 2024
5 tasks
@Wumpf Wumpf self-requested a review March 4, 2024 15:39
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

this looks pretty nice for a c++ pr 😁


bool is_file = std::filesystem::is_regular_file(filepath);
bool is_cpp_file = std::filesystem::path(filepath).extension().string() == ".cpp";

// Inform the Rerun Viewer that we do not support that kind of file.
if (!is_file || is_cpp_file) {
if (!is_file || !is_cpp_file) {
Copy link
Member

Choose a reason for hiding this comment

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

oops, how did that go through before

Base automatically changed from cmc/sdk_dataloader_7_stateful_log_file to main March 4, 2024 16:45
teh-cmc added a commit that referenced this pull request Mar 4, 2024
This makes the `log_file` APIs behave more like the standard `log` APIs;
i.e. they inherit the state of their associated `RecordingStream`
(app_id, rec_id, timepoint, etc...).

Also inherit the application ID while we're at it.

Makes the API much nicer to use _and_ much more consistent with the
rest.

Checks:
- [x] external loader ran manually (`python loader | rerun`)
- [x] external loader via rerun (`rerun xxx.py`)
- [x] log_file with external loader (`log_file xxx.py`)
- [x] external loader ran manually (`loader | rerun`)
- [x] external loader via rerun (`rerun xxx.rs`)
- [x] log_file with external loader (`log_file xxx.rs`)

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5379
- #5361
- #5388

---------

Co-authored-by: Andreas Reich <andreas@rerun.io>
@teh-cmc teh-cmc force-pushed the cmc/sdk_dataloader_7_cpp_settings branch from d6c346c to 25b281a Compare March 4, 2024 16:46
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Mar 4, 2024
@teh-cmc teh-cmc merged commit 8790e6f into main Mar 4, 2024
21 of 31 checks passed
@teh-cmc teh-cmc deleted the cmc/sdk_dataloader_7_cpp_settings branch March 4, 2024 16:47
teh-cmc added a commit that referenced this pull request Mar 6, 2024
I guess that's good enough 🤷. I don't know, my brain has been
completely friend by C++ non-sense all day.

This includes a fix to make sure that a viewer that was spawned from the
python SDK is still allowed to spawn dataloaders implemented in python
(`RERUN_APP_ONLY` shenaniganeries).

- Fixes #4526 

---

Part of series of PR to expose configurable `DataLoader`s to our SDKs:
- #5327 
- #5328 
- #5330
- #5337
- #5351
- #5355
- #5379
- #5361
- #5388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 C++ API C or C++ logging API include in changelog 🪵 Log-API Affects the user-facing API for all languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants