Skip to content

[C++] Rename rerun::ComponentBatch to rerun::Collection (and related constructs)#4225

Closed
Wumpf wants to merge 5 commits intomainfrom
andreas/cpp/rerun-collection
Closed

[C++] Rename rerun::ComponentBatch to rerun::Collection (and related constructs)#4225
Wumpf wants to merge 5 commits intomainfrom
andreas/cpp/rerun-collection

Conversation

@Wumpf
Copy link
Copy Markdown
Member

@Wumpf Wumpf commented Nov 14, 2023

What

Not much else has changed yet except some doc massaging. First step towards solving

rerun::Collection will evolve from here on and be the corner piece of our zero copy & component adaptability strategy (just like ComponentBatch is already!)

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 demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@Wumpf Wumpf added 🚜 refactor Change the code, not the functionality sdk-cpp C/C++ API specific include in changelog labels Nov 14, 2023
@Wumpf Wumpf force-pushed the andreas/cpp/rerun-collection branch from 734f5dc to 1c08991 Compare November 14, 2023 15:45
#include "bar_chart.hpp"

// #define EDIT_EXTENSION
namespace rerun::archetypes {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can this be added to the clang-format file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only found https://clang.llvm.org/docs/ClangFormatStyleOptions.html#compactnamespaces which isn't quite the same

But as you pointed out elsewhere there's also the option to not indent after namespace, we should try that (in a separate pr)
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#namespaceindentation

}

// We want to change the dimension names if they are not specified.
// But rerun collections are strictly immutable, so create a new one if necessary.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we sure using Collection for TensorDimension makes sense? When will it avoid allocations and memcpys, realistically?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's less about avoiding copies & allocs but more about being able to use the adapters for this type as well

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Nov 15, 2023

oooops, that was the wrong source branch 😳 😳. The plan was to have a PR only for up to 3de46c8
The rest is still super experimental. Mopping up. Since this was already approved now I merge the rename pr and bring up a new one for the rest later!

@Wumpf Wumpf closed this Nov 15, 2023
Wumpf added a commit that referenced this pull request Nov 15, 2023
…ted constructs) (#4236)

### What

Not much else has changed yet except some doc massaging. First step
towards solving

* #3794

`rerun::Collection` will evolve from here on and be the corner piece of
our zero copy & component adaptability strategy (just like
ComponentBatch is already!)


_This was originally opened on the wrong branch by accident:_
*  #4225

### 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)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4225) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4225)
- [Docs
preview](https://rerun.io/preview/3de46c8a5d294eb5ad68e5beb35a64d0610e992a/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/3de46c8a5d294eb5ad68e5beb35a64d0610e992a/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
@teh-cmc teh-cmc mentioned this pull request Dec 12, 2023
18 tasks
@Wumpf Wumpf deleted the andreas/cpp/rerun-collection branch January 10, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

include in changelog 🚜 refactor Change the code, not the functionality sdk-cpp C/C++ API specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants