Skip to content

Create recording2mcap and dummy_data tools #30

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

Merged
merged 10 commits into from
Nov 7, 2024
Merged

Conversation

jaagut
Copy link
Member

@jaagut jaagut commented Oct 31, 2024

This pull request introduces several enhancements and new features across various files, focusing on dummy data generation, and MCAP file conversion. The most important changes include renaming a workflow file, adding new functions for dummy data insertion, and implementing a new script for converting recordings to MCAP format.

Workflow and Configuration Updates:

  • Renamed .github/workflows/create_db.yml to .github/workflows/dummy_db.yml and updated the workflow to include dummy data population steps. (.github/workflows/dummy_db.yml) [1] [2]

Dummy Data Insertion:

  • Added new functions in ddlitlab2024/dataset/dummy_data.py to insert dummy data into the database, including recordings, images, rotations, joint states, joint commands, and game states. (ddlitlab2024/dataset/dummy_data.py)

MCAP File Conversion:

  • Implemented a new script recording2mcap.py for converting recordings to MCAP format, including functions to write images, rotations, joint states, joint commands, and game states to MCAP files. (ddlitlab2024/dataset/recording2mcap.py)

Miscellaneous Updates:

  • Added new words to the spell checker configuration in .vscode/settings.json. (.vscode/settings.json)
  • Added a new layout configuration file lichtblick_layout.json. (ddlitlab2024/dataset/lichtblick_layout.json)
  • Added a new utility function stamp_to_seconds_nanoseconds to convert timestamps. (ddlitlab2024/dataset/models.py)
  • Removed unused SQLite connection code from reader.py. (ddlitlab2024/dataset/reader.py)
  • Added mcap dependency to pyproject.toml. (pyproject.toml)

@jaagut jaagut requested review from texhnolyze and Flova October 31, 2024 20:10
@jaagut jaagut self-assigned this Oct 31, 2024
if not input(f"Output directory '{output}' already exists. Overwrite? (y/n): ").lower().startswith("y"):
logger.info("Exiting")
sys.exit(0)
# Remove the existing directory
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think comments like these are helpful?
I'm personally not a big proponent of comments in general, and think they should only ever be used to explain why something is done and not what is done (especially when it is obvious)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do these comments bother you? Why?

I agree, describing the Why is more important (and often missing) than the What.
But I still see value in these comments, as they describe the intent of the code block. That means, I can quickly understand the flow and actions of the code without reading code which is noisier and requires more experience. Reading one comment spares me from parsing one block in my head.

In this case:
I very rarely use shutil and rmtree is not the most intuitive name. Of course, I could assume its function considering its input, but when still unsure it requires to read the docs.

Alternatively, one could demand each code-block with such a comment should be its method with proper documentation. This seems silly for a one-liner. For longer blocks, I would normally agree, but this is just a small utility tool, where I don't think it's worth the effort. That's why I use these comments as a middle ground between proper clean functions or nothing.

@jaagut jaagut requested a review from texhnolyze November 7, 2024 11:34
@texhnolyze texhnolyze merged commit 8b7e3d2 into main Nov 7, 2024
2 checks passed
@texhnolyze texhnolyze deleted the feature/viz_dataset branch November 7, 2024 11:46
Flova pushed a commit that referenced this pull request Apr 21, 2025
Create recording2mcap and dummy_data tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants