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.

Sign up for GitHub

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

Jump to bottom

New TextLog archetype #3261

Merged
merged 28 commits into from
Sep 10, 2023
Merged

New TextLog archetype #3261

merged 28 commits into from
Sep 10, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Sep 8, 2023 •
edited
Loading

What

Adds

  • component TextLogLevel
  • archetype TextLog

Checklist

@Wumpf Wumpf self-requested a review September 8, 2023 09:50
Wumpf
Wumpf approved these changes Sep 8, 2023
View reviewed changes
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.

lgtm but I don't understand the change in python to string literals everywhere - there is documentation and attributes set on TextLogLevelType but then they aren't used anywhere?

crates/re_data_ui/src/component_ui_registry.rs Show resolved Hide resolved
docs/content/reference/data_types.md
@@ -39,5 +39,4 @@ generic container for arrays of data. Images are restricted to tensors of rank 2

## Other **Archetypes**
* [Scalar](data_types/scalar.md): a single scalar / metric value. Can be viewed in the `TimeSeries` space view.
* [TextEntry](data_types/text_entry.md): captures text data. Can be viewed in the `Text` space view.
Copy link
Member

Choose a reason for hiding this comment

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

bit unfortunate to throw it away entirely and thus any mention of the ability to log text.
But I suppose putting up the new doc pages with all the examples and generated images is orthogonal

rerun_cpp/src/rerun/components/class_id.cpp Show resolved Hide resolved
examples/python/signed_distance_fields/main.py
@@ -79,21 +79,21 @@ def read_mesh(path: Path) -> Trimesh:
return cast(Trimesh, mesh)


@log_timing_decorator("global/voxel_sdf", rr.LogLevel.DEBUG) # type: ignore[misc]
@log_timing_decorator("global/voxel_sdf", "DEBUG") # type: ignore[misc]
Copy link
Member

Choose a reason for hiding this comment

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

here and in all other python example code: Why not use constants for the level strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Truth? Because I am fed up with Python and so I threw my hands in the air and said "what's the difference - the errors are only caught at runtime anyway - at least a string literal is short"

Copy link
Member

Choose a reason for hiding this comment

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

😢

Copy link
Member Author

Choose a reason for hiding this comment

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

…also I thought I hade opened this PR in draft mode 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

I also don't know how to do it. rr2.components.TextLogLevelType.INFO doesn't work: module 'rerun.components' has no attribute 'TextLogLevelType'

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll fix it in a later PR

@emilk emilk merged commit 13b5039 into main Sep 10, 2023
@emilk emilk deleted the emilk/text-log branch September 10, 2023 17:28
@emilk emilk added the exclude from changelog PRs with this won't show up in CHANGELOG.md label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wumpf Wumpf Wumpf approved these changes

Assignees
No one assigned
Labels
codegen/idl exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emilk @Wumpf