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

rerun print: print just summary, unless given --verbose #5079

Merged
merged 5 commits into from
Feb 7, 2024
Merged

rerun print: print just summary, unless given --verbose #5079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3482ae9
`rerun print`: print just summary, unless given `--verbose`
emilk Feb 7, 2024
7749a55
Shorter debug-format of PythonVersion
emilk Feb 7, 2024
ac3fe92
Fix: "kiB" -> "KiB"
emilk Feb 7, 2024
1eff9e5
Call into Display fmt
emilk Feb 7, 2024
5077a74
Add note about case-sensitivity
emilk Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions crates/re_format/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn test_format_large_number() {
/// ```
/// # use re_format::format_bytes;
/// assert_eq!(format_bytes(123.0), "123 B");
/// assert_eq!(format_bytes(12_345.0), "12.1 kiB");
/// assert_eq!(format_bytes(12_345.0), "12.1 KiB");
/// assert_eq!(format_bytes(1_234_567.0), "1.2 MiB");
/// assert_eq!(format_bytes(123_456_789.0), "118 MiB");
/// ```
Expand All @@ -147,7 +147,7 @@ pub fn format_bytes(number_of_bytes: f64) -> String {
format!("{number_of_bytes:.0} B")
} else if number_of_bytes < 20.0_f64.exp2() {
let decimals = (10.0 * number_of_bytes < 20.0_f64.exp2()) as usize;
format!("{:.*} kiB", decimals, number_of_bytes / 10.0_f64.exp2())
format!("{:.*} KiB", decimals, number_of_bytes / 10.0_f64.exp2())
} else if number_of_bytes < 30.0_f64.exp2() {
let decimals = (10.0 * number_of_bytes < 30.0_f64.exp2()) as usize;
format!("{:.*} MiB", decimals, number_of_bytes / 20.0_f64.exp2())
Expand All @@ -164,12 +164,12 @@ fn test_format_bytes() {
(1000.0, "1000 B"),
(1001.0, "1001 B"),
(1023.0, "1023 B"),
(1024.0, "1.0 kiB"),
(1025.0, "1.0 kiB"),
(1024.0 * 1.2345, "1.2 kiB"),
(1024.0 * 12.345, "12.3 kiB"),
(1024.0 * 123.45, "123 kiB"),
(1024f64.powi(2) - 1.0, "1024 kiB"),
(1024.0, "1.0 KiB"),
(1025.0, "1.0 KiB"),
(1024.0 * 1.2345, "1.2 KiB"),
(1024.0 * 12.345, "12.3 KiB"),
(1024.0 * 123.45, "123 KiB"),
(1024f64.powi(2) - 1.0, "1024 KiB"),
(1024f64.powi(2) + 0.0, "1.0 MiB"),
(1024f64.powi(2) + 1.0, "1.0 MiB"),
(1024f64.powi(3) - 1.0, "1024 MiB"),
Expand All @@ -182,7 +182,7 @@ fn test_format_bytes() {
(1024f64.powi(4) + 0.0, "1024 GiB"),
(1024f64.powi(4) + 1.0, "1024 GiB"),
(123.0, "123 B"),
(12_345.0, "12.1 kiB"),
(12_345.0, "12.1 KiB"),
(1_234_567.0, "1.2 MiB"),
(123_456_789.0, "118 MiB"),
];
Expand All @@ -193,6 +193,7 @@ fn test_format_bytes() {
}

pub fn parse_bytes_base10(bytes: &str) -> Option<i64> {
// Note: intentionally case sensitive so that we don't parse `Mb` (Megabit) as `MB` (Megabyte).
if let Some(kb) = bytes.strip_suffix("kB") {
Some(kb.parse::<i64>().ok()? * 1_000)
} else if let Some(mb) = bytes.strip_suffix("MB") {
Expand Down Expand Up @@ -231,7 +232,8 @@ fn test_parse_bytes_base10() {
}

pub fn parse_bytes_base2(bytes: &str) -> Option<i64> {
Copy link
Member

Choose a reason for hiding this comment

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

isn't that the function we use to parse user input in env variables? i'm a bit worried to see it being case-sensitive, that's gonna trip up a lot of people...

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 doubt anyone is specifying a memory limit in KiB, so I think this small breaking change can slip though.

As for case insensitive: I don't think we should parse Mib (Mebi-bits) as MiB (Mebi-bytes). that could trip someone up.

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 added a comment about it

if let Some(kb) = bytes.strip_suffix("kiB") {
// Note: intentionally case sensitive so that we don't parse `Mib` (Mebibit) as `MiB` (Mebibyte).
if let Some(kb) = bytes.strip_suffix("KiB") {
Some(kb.parse::<i64>().ok()? * 1024)
} else if let Some(mb) = bytes.strip_suffix("MiB") {
Some(mb.parse::<i64>().ok()? * 1024 * 1024)
Expand All @@ -252,16 +254,16 @@ fn test_parse_bytes_base2() {
("999B", 999),
("1023B", 1_023),
("1024B", 1_024),
("1kiB", 1_024),
("1000kiB", 1_000 * 1024),
("1KiB", 1_024),
("1000KiB", 1_000 * 1024),
("1MiB", 1024 * 1024),
("1000MiB", 1_000 * 1024 * 1024),
("1GiB", 1024 * 1024 * 1024),
("1000GiB", 1_000 * 1024 * 1024 * 1024),
("1TiB", 1024 * 1024 * 1024 * 1024),
("1000TiB", 1_000 * 1024 * 1024 * 1024 * 1024),
("123B", 123),
("12kiB", 12 * 1024),
("12KiB", 12 * 1024),
("123MiB", 123 * 1024 * 1024),
];
for (value, expected) in test_cases {
Expand Down Expand Up @@ -294,16 +296,16 @@ fn test_parse_bytes() {
("999B", 999),
("1023B", 1_023),
("1024B", 1_024),
("1kiB", 1_024),
("1000kiB", 1_000 * 1024),
("1KiB", 1_024),
("1000KiB", 1_000 * 1024),
("1MiB", 1024 * 1024),
("1000MiB", 1_000 * 1024 * 1024),
("1GiB", 1024 * 1024 * 1024),
("1000GiB", 1_000 * 1024 * 1024 * 1024),
("1TiB", 1024 * 1024 * 1024 * 1024),
("1000TiB", 1_000 * 1024 * 1024 * 1024 * 1024),
("123B", 123),
("12kiB", 12 * 1024),
("12KiB", 12 * 1024),
("123MiB", 123 * 1024 * 1024),
];
for (value, expected) in test_cases {
Expand Down
8 changes: 7 additions & 1 deletion crates/re_log_types/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl StoreInfo {
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct PythonVersion {
/// e.g. 3
Expand All @@ -288,6 +288,12 @@ pub struct PythonVersion {
pub suffix: String,
}

impl std::fmt::Debug for PythonVersion {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self, f)
}
}

impl std::fmt::Display for PythonVersion {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Self {
Expand Down
80 changes: 59 additions & 21 deletions crates/rerun/src/run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use re_smart_channel::{ReceiveSet, Receiver, SmartMessagePayload};

#[cfg(feature = "web_viewer")]
use re_sdk::web_viewer::host_web_viewer;
use re_types::SizeBytes;
#[cfg(feature = "web_viewer")]
use re_web_viewer_server::WebViewerServerPort;
#[cfg(feature = "server")]
Expand Down Expand Up @@ -241,7 +242,7 @@ enum Command {
},

/// Print the contents of an .rrd file.
Print { rrd_path: String },
Print(PrintCommand),

/// Reset the memory of the Rerun Viewer.
///
Expand All @@ -253,6 +254,15 @@ enum Command {
Reset,
}

#[derive(Debug, Clone, clap::Parser)]
struct PrintCommand {
rrd_path: String,

/// If specified, print out table contents.
#[clap(long, short, default_value_t = false)]
verbose: bool,
}

#[derive(Debug, Clone, Subcommand)]
enum AnalyticsCommands {
/// Prints extra information about analytics.
Expand Down Expand Up @@ -368,10 +378,7 @@ where
run_compare(&path_to_rrd1, &path_to_rrd2, *full_dump)
}

Command::Print { rrd_path } => {
let rrd_path = PathBuf::from(&rrd_path);
print_rrd(&rrd_path).with_context(|| format!("path: {rrd_path:?}"))
}
Command::Print(print_command) => print_command.run(),

#[cfg(feature = "native_viewer")]
Command::Reset => reset_viewer(),
Expand Down Expand Up @@ -486,25 +493,56 @@ fn run_compare(path_to_rrd1: &Path, path_to_rrd2: &Path, full_dump: bool) -> any
re_log_types::DataTable::similar(&table1, &table2)
}

fn print_rrd(rrd_path: &Path) -> anyhow::Result<()> {
let rrd_file = std::fs::File::open(rrd_path)?;
let version_policy = re_log_encoding::decoder::VersionPolicy::Error;
let decoder = re_log_encoding::decoder::Decoder::new(version_policy, rrd_file)?;
for msg in decoder {
let msg = msg.context("decode rrd message")?;
match msg {
LogMsg::SetStoreInfo(msg) => {
let SetStoreInfo { row_id: _, info } = msg;
println!("{info:#?}");
}
LogMsg::ArrowMsg(_row_id, arrow_msg) => {
let table =
DataTable::from_arrow_msg(&arrow_msg).context("Decode arrow message")?;
println!("{table}");
impl PrintCommand {
fn run(&self) -> anyhow::Result<()> {
let rrd_path = PathBuf::from(&self.rrd_path);
self.print_rrd(&rrd_path)
.with_context(|| format!("path: {rrd_path:?}"))
}

fn print_rrd(&self, rrd_path: &Path) -> anyhow::Result<()> {
let Self {
rrd_path: _,
verbose,
} = self;

let rrd_file = std::fs::File::open(rrd_path)?;
let version_policy = re_log_encoding::decoder::VersionPolicy::Warn;
let decoder = re_log_encoding::decoder::Decoder::new(version_policy, rrd_file)?;
for msg in decoder {
let msg = msg.context("decode rrd message")?;
match msg {
LogMsg::SetStoreInfo(msg) => {
let SetStoreInfo { row_id: _, info } = msg;
println!("{info:#?}");
}
LogMsg::ArrowMsg(_row_id, arrow_msg) => {
let mut table =
DataTable::from_arrow_msg(&arrow_msg).context("Decode arrow message")?;

if *verbose {
println!("{table}");
} else {
table.compute_all_size_bytes();

let column_names = table
.columns
.keys()
.map(|name| name.short_name())
.collect_vec();

println!(
"Table with {} rows ({}). Columns: {:?}",
table.num_rows(),
re_format::format_bytes(table.heap_size_bytes() as _),
column_names
);
}
}
}
}
Ok(())
}
Ok(())
}

#[cfg(feature = "analytics")]
Expand Down
Loading