diff --git a/.claude/skills/dataprof/reference/api.md b/.claude/skills/dataprof/reference/api.md index 531f8d8b..56e759e6 100644 --- a/.claude/skills/dataprof/reference/api.md +++ b/.claude/skills/dataprof/reference/api.md @@ -37,7 +37,7 @@ Keyword arguments: `engine`, `chunk_size`, `memory_limit_mb`, `format`, `max_rows`, `name`, `csv_delimiter`, `csv_flexible`, `jsonl_on_error`, `sampling`, `stop_condition`, `on_progress`, `progress_interval_ms`, `quality_dimensions`, `metrics`, `locale`, `positive_columns`, -`identifier_columns`, `temporal_columns`. +`identifier_columns`, `temporal_columns`, `temporal_pairs`. ## Profiler builder (when keyword arguments are not enough) @@ -56,7 +56,8 @@ report = ( Every builder method returns the profiler, so calls chain. Other methods: `chunk_size`, `memory_limit_mb`, `format`, `name`, `csv_delimiter`, `csv_flexible`, `sampling`, `stop_condition`, `stop_when`, `on_progress`, -`progress_interval_ms`, `locale`, `positive_columns`, `temporal_columns`. +`progress_interval_ms`, `locale`, `positive_columns`, `temporal_columns`, +`temporal_pairs`. ## ProfileReport: execution metadata diff --git a/.claude/skills/dataprof/reference/interpretation.md b/.claude/skills/dataprof/reference/interpretation.md index f2363940..f598dbf0 100644 --- a/.claude/skills/dataprof/reference/interpretation.md +++ b/.claude/skills/dataprof/reference/interpretation.md @@ -181,7 +181,8 @@ with every locale-specific pattern suppressed. `before.compare(after)` returns deltas. Three caveats: - Comparing a sampled profile to a full one measures your sampling, not drift. - Check `sampling_applied` on both sides first. + Each score's `evidence` and `reports.a`/`reports.b` `evidence` say whether a + side was truncated, sampled or scored from a sample; read them first. - Comparing profiles with different `metrics` selections produces absence, not change. Narrow both sides the same way. - A stored report can predate a change in how something is measured: text diff --git a/crates/dataprof-core/src/progress.rs b/crates/dataprof-core/src/progress.rs index 7bc9a7d1..7b07a40b 100644 --- a/crates/dataprof-core/src/progress.rs +++ b/crates/dataprof-core/src/progress.rs @@ -52,6 +52,17 @@ impl ProgressSink { !matches!(self, Self::None) } + /// Reserve channel capacity for a terminal event without changing the + /// public shape of the Channel variant. + #[doc(hidden)] + #[cfg(feature = "async-streaming")] + pub fn reserve_terminal(&self) -> Option> { + match self { + Self::Channel(tx) => tx.clone().try_reserve_owned().ok(), + _ => None, + } + } + /// Deliver one event. A full channel drops it rather than block profiling. pub fn emit(&self, event: ProgressEvent) { match self { diff --git a/crates/dataprof-core/src/semantic.rs b/crates/dataprof-core/src/semantic.rs index ece6f83f..b547856f 100644 --- a/crates/dataprof-core/src/semantic.rs +++ b/crates/dataprof-core/src/semantic.rs @@ -12,6 +12,8 @@ pub enum SemanticHintKind { Identifier, /// A [`SemanticHints::temporal_columns`] entry. Temporal, + /// A column named by a [`SemanticHints::temporal_pairs`] entry. + TemporalPair, } impl SemanticHintKind { @@ -21,6 +23,7 @@ impl SemanticHintKind { Self::Positive => "positive_columns", Self::Identifier => "identifier_columns", Self::Temporal => "temporal_columns", + Self::TemporalPair => "temporal_pairs", } } } @@ -66,6 +69,8 @@ pub struct SemanticHints { pub positive_columns: Vec, pub identifier_columns: Vec, pub temporal_columns: Vec, + /// Start/end column pairs declared by the caller, compared as given. + pub temporal_pairs: Vec<(String, String)>, } impl SemanticHints { @@ -74,6 +79,7 @@ impl SemanticHints { positive_columns, identifier_columns, temporal_columns: Vec::new(), + temporal_pairs: Vec::new(), } } @@ -82,10 +88,18 @@ impl SemanticHints { self } + /// Declare start/end column pairs for timeliness to compare as given, + /// such as `valid_from`/`valid_until`, which no role word pairs by name. + pub fn with_temporal_pairs(mut self, temporal_pairs: Vec<(String, String)>) -> Self { + self.temporal_pairs = temporal_pairs; + self + } + pub fn is_empty(&self) -> bool { self.positive_columns.is_empty() && self.identifier_columns.is_empty() && self.temporal_columns.is_empty() + && self.temporal_pairs.is_empty() } pub fn is_identifier_column(&self, column_name: &str) -> bool { @@ -106,7 +120,21 @@ impl SemanticHints { .any(|candidate| candidate == column_name) } - /// Every `(kind, column_name)` pair across all three hint lists. + /// Each column named by a declared pair, once, in the order first named. + pub fn temporal_pair_columns(&self) -> Vec<&str> { + let mut columns: Vec<&str> = Vec::new(); + for (start, end) in &self.temporal_pairs { + for name in [start.as_str(), end.as_str()] { + if !columns.contains(&name) { + columns.push(name); + } + } + } + columns + } + + /// Every `(kind, column_name)` pair across the hint lists, with each column + /// of a declared pair listed once. pub fn iter(&self) -> impl Iterator { let positive = self .positive_columns @@ -120,7 +148,11 @@ impl SemanticHints { .temporal_columns .iter() .map(|c| (SemanticHintKind::Temporal, c.as_str())); - positive.chain(identifier).chain(temporal) + let pairs = self + .temporal_pair_columns() + .into_iter() + .map(|c| (SemanticHintKind::TemporalPair, c)); + positive.chain(identifier).chain(temporal).chain(pairs) } /// Fail loudly when a hint names a column that is not in the schema. @@ -129,6 +161,14 @@ impl SemanticHints { /// hint that cannot bind must not vanish: a typo'd or stale column name is a /// mistake the profiler should surface, not silently discard. pub fn validate_names(&self, column_names: &[&str]) -> Result<(), DataProfilerError> { + // A column compared with itself can never be out of order, so such a + // pair would report a clean check that never happened. + if let Some((name, _)) = self.temporal_pairs.iter().find(|(start, end)| start == end) { + return Err(DataProfilerError::InvalidSemanticHint { + message: format!("temporal_pairs names '{name}' as both start and end"), + suggestion: "Pair a start column with a different end column.".to_string(), + }); + } let mut unknown: Vec<(SemanticHintKind, &str)> = self .iter() .filter(|(_, name)| !column_names.contains(name)) @@ -167,14 +207,17 @@ impl SemanticHints { /// effective while producing neither metrics nor binding evidence. pub fn validate_quality_usage(&self, quality_computed: bool) -> Result<(), DataProfilerError> { if quality_computed - || (self.positive_columns.is_empty() && self.temporal_columns.is_empty()) + || (self.positive_columns.is_empty() + && self.temporal_columns.is_empty() + && self.temporal_pairs.is_empty()) { return Ok(()); } Err(DataProfilerError::InvalidSemanticHint { - message: "positive_columns and temporal_columns require the Quality metric pack" - .to_string(), + message: + "positive_columns, temporal_columns and temporal_pairs require the Quality metric pack" + .to_string(), suggestion: "Include MetricPack::Quality (\"quality\" in Python), or remove the value-driven hint. Identifier hints remain usable without quality because they affect column typing." .to_string(), }) @@ -210,9 +253,11 @@ impl SemanticHints { Err(DataProfilerError::InvalidSemanticHint { message: format!("semantic hint(s) bound to no matching value: {details}"), - suggestion: "positive_columns need numeric values; temporal_columns need date values. \ + suggestion: + "positive_columns need numeric values; temporal_columns and temporal_pairs \ + need date values. \ Fix the column choice or remove the hint." - .to_string(), + .to_string(), }) } } diff --git a/crates/dataprof-db/src/connectors/sqlite.rs b/crates/dataprof-db/src/connectors/sqlite.rs index 6741a3b3..8c847562 100644 --- a/crates/dataprof-db/src/connectors/sqlite.rs +++ b/crates/dataprof-db/src/connectors/sqlite.rs @@ -69,11 +69,20 @@ impl DatabaseConnector for SqliteConnector { async fn connect(&mut self) -> Result<(), DataProfilerError> { #[cfg(feature = "sqlite")] { - let db_path = self.get_db_path()?; - let connection_string = if db_path == ":memory:" { - "sqlite::memory:".to_string() + // sqlx parses SQLite-specific URL options (mode, cache, immutable, + // vfs). Passing the original sqlite: URI through unchanged also + // preserves its absolute/relative path semantics (#912 / #839). + // Previously rebuilding the URL from get_db_path() silently + // dropped every query parameter, including mode=ro and mode=rwc. + let connection_string = if self.config.connection_string.starts_with("sqlite:") { + self.config.connection_string.clone() } else { - format!("sqlite://{}", db_path) + let db_path = self.get_db_path()?; + if db_path == ":memory:" { + "sqlite::memory:".to_string() + } else { + format!("sqlite://{}", db_path) + } }; let pool = SqlitePoolOptions::new() diff --git a/crates/dataprof-db/src/lib.rs b/crates/dataprof-db/src/lib.rs index 3a1de791..a315f651 100644 --- a/crates/dataprof-db/src/lib.rs +++ b/crates/dataprof-db/src/lib.rs @@ -144,7 +144,11 @@ fn apply_environment_configuration( .apply_to_connection_string(config.connection_string.clone(), database_type); } - if config.load_credentials_from_env { + if config.load_credentials_from_env && database_type != "sqlite" { + // An explicit SQLite URI is a local file target, not a consumer of + // generic DATABASE_URL / DB_* credentials from other engines. + // Rebuilding it with foreign settings may replace its path or + // introduce unsupported user-info/host components (#912). let credentials = DatabaseCredentials::from_environment(database_type); config.connection_string = credentials.apply_to_connection_string(&config.connection_string); @@ -191,8 +195,8 @@ pub async fn analyze_database_with_options( // ignored. Checked before connecting so nothing is scanned first. if !options.semantic_hints().is_empty() { return Err(DataProfilerError::UnsupportedDataSource { - message: "positive_columns, identifier_columns, and temporal_columns are not \ - supported for database profiling yet" + message: "positive_columns, identifier_columns, temporal_columns, and temporal_pairs \ + are not supported for database profiling yet" .to_string(), }); } @@ -280,10 +284,16 @@ pub async fn analyze_database_with_options( // from value-level checks, and the columns no longer line up. let mut uniqueness = RowUniquenessTracker::default(); if !aligned.is_empty() { + let container_rows: Vec<&[usize]> = columns.container_rows().collect(); for row_index in 0..actual_rows_processed { let mut signature = RowSignature::default(); - for cells in &aligned { - signature.push_field(&cells[row_index]); + for (cells, containers) in aligned.iter().zip(&container_rows) { + // Ascending by construction: rows are pushed in order. + if containers.binary_search(&row_index).is_ok() { + signature.push_container_field(&cells[row_index]); + } else { + signature.push_field(&cells[row_index]); + } } uniqueness.observe(signature.finish()); } diff --git a/crates/dataprof-db/src/query_columns.rs b/crates/dataprof-db/src/query_columns.rs index 0accbe83..c6e03d88 100644 --- a/crates/dataprof-db/src/query_columns.rs +++ b/crates/dataprof-db/src/query_columns.rs @@ -18,9 +18,9 @@ use std::ops::Index; #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct QueryColumns { columns: Vec<(String, Vec)>, - /// Per column, by position, how many values were a JSON object or array or - /// a SQL array. Always as long as `columns`. - containers: Vec, + /// Per column, by position, the rows whose value was a JSON object or array + /// or a SQL array, ascending. Always as long as `columns`. + containers: Vec>, } impl QueryColumns { @@ -58,8 +58,10 @@ impl QueryColumns { /// container: a JSON object or array, or a SQL array. pub fn push_decoded(&mut self, index: usize, value: String, container: bool) { if let Some((_, data)) = self.columns.get_mut(index) { + if container { + self.containers[index].push(data.len()); + } data.push(value); - self.containers[index] += usize::from(container); } } @@ -69,11 +71,18 @@ impl QueryColumns { self.columns .iter() .zip(&self.containers) - .filter(|(_, count)| **count > 0) - .map(|((name, _), count)| (name.clone(), *count)) + .filter(|(_, rows)| !rows.is_empty()) + .map(|((name, _), rows)| (name.clone(), rows.len())) .collect() } + /// Per column, in query order, the rows whose value was a container, + /// ascending. Row signatures tag those values apart from a string that + /// spells the same text (#945). + pub fn container_rows(&self) -> impl Iterator { + self.containers.iter().map(Vec::as_slice) + } + /// Number of columns. pub fn len(&self) -> usize { self.columns.len() @@ -155,7 +164,7 @@ impl Index<&str> for QueryColumns { impl FromIterator<(String, Vec)> for QueryColumns { fn from_iter)>>(iter: I) -> Self { let columns: Vec<_> = iter.into_iter().collect(); - let containers = vec![0; columns.len()]; + let containers = vec![Vec::new(); columns.len()]; Self { columns, containers, diff --git a/crates/dataprof-db/tests/sqlite_url_options.rs b/crates/dataprof-db/tests/sqlite_url_options.rs new file mode 100644 index 00000000..2121dde3 --- /dev/null +++ b/crates/dataprof-db/tests/sqlite_url_options.rs @@ -0,0 +1,134 @@ +//! #912: SQLite URI query parameters must not be stripped before sqlx sees +//! them. These are behavioral tests: they exercise actual read-only and +//! create-if-missing modes, not only string reconstruction. +#![cfg(feature = "sqlite")] + +use dataprof_db::{DatabaseConfig, create_connector}; +use sqlx::sqlite::SqlitePoolOptions; + +fn config(url: String) -> DatabaseConfig { + DatabaseConfig { + connection_string: url, + load_credentials_from_env: false, + ..Default::default() + } +} + +fn sqlite_url(path: &std::path::Path, mode: &str) -> String { + // sqlx accepts sqlite:///abs/path on Unix and sqlite:///C:/path on + // Windows. A slash is preserved rather than trimming URL path segments. + let path = path.to_string_lossy().replace('\\', "/"); + let prefix = if cfg!(windows) { + "sqlite:///" + } else { + "sqlite://" + }; + format!("{prefix}{path}?mode={mode}") +} + +#[tokio::test] +async fn mode_rwc_creates_a_missing_database() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("created-by-mode-rwc.db"); + assert!(!path.exists()); + + let mut connector = create_connector(config(sqlite_url(&path, "rwc"))).unwrap(); + connector.connect().await.unwrap(); + assert!(connector.test_connection().await.unwrap()); + assert!(path.exists(), "SQLite must honor mode=rwc, not drop it"); + connector.disconnect().await.unwrap(); +} + +#[tokio::test] +async fn mode_rw_does_not_create_a_missing_database() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("must-not-be-created.db"); + assert!(!path.exists()); + + let mut connector = create_connector(config(sqlite_url(&path, "rw"))).unwrap(); + assert!( + connector.connect().await.is_err(), + "mode=rw must reject a missing DB rather than silently create it" + ); + assert!(!path.exists()); +} + +#[tokio::test] +async fn mode_ro_refuses_writes_to_existing_database() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("read-only.db"); + + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect(&sqlite_url(&path, "rwc")) + .await + .unwrap(); + sqlx::query("CREATE TABLE rows (id INTEGER)") + .execute(&pool) + .await + .unwrap(); + pool.close().await; + + let mut connector = create_connector(config(sqlite_url(&path, "ro"))).unwrap(); + connector.connect().await.unwrap(); + assert!(connector.test_connection().await.unwrap()); + let error = connector + .profile_query("INSERT INTO rows (id) VALUES (1)") + .await + .expect_err("mode=ro must reject writes to the database"); + assert!( + error.to_string().to_ascii_lowercase().contains("readonly") + || error.to_string().to_ascii_lowercase().contains("read-only"), + "unexpected SQLite error: {error}" + ); + connector.disconnect().await.unwrap(); +} + +/// Run in a separate test process so hostile DATABASE_URL values never leak +/// into other Rust tests (the process environment is global). +#[tokio::test] +async fn sqlite_explicit_url_default_env_child() { + if std::env::var_os("DATAPROF_SQLITE_ENV_CHILD").is_none() { + return; + } + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("explicit-default-rwc.db"); + let url = sqlite_url(&path, "rwc"); + // Unlike the other tests, use the public default setting: + // load_credentials_from_env = true. A PostgreSQL DATABASE_URL must not + // replace the SQLite path or its explicitly supplied mode. + let mut connector = create_connector(DatabaseConfig { + connection_string: url, + ..Default::default() + }) + .unwrap(); + connector.connect().await.unwrap(); + assert!(path.exists(), "the requested SQLite path must be created"); + assert!(connector.test_connection().await.unwrap()); + connector.disconnect().await.unwrap(); +} + +#[test] +fn explicit_sqlite_mode_survives_unrelated_database_url() { + let executable = std::env::current_exe().expect("locate Rust test executable"); + let output = std::process::Command::new(executable) + .args([ + "--exact", + "sqlite_explicit_url_default_env_child", + "--nocapture", + ]) + .env("DATAPROF_SQLITE_ENV_CHILD", "1") + .env( + "DATABASE_URL", + "postgresql://unrelated:unrelated@127.0.0.1:1/unrelated_database", + ) + .output() + .expect("execute isolated default-configuration regression"); + assert!( + output.status.success() && String::from_utf8_lossy(&output.stdout).contains("1 passed"), + "an unrelated DATABASE_URL must not hijack an explicit SQLite target:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/crates/dataprof-db/tests/structured_column_decoding.rs b/crates/dataprof-db/tests/structured_column_decoding.rs index 41c83bbd..ae1dd44b 100644 --- a/crates/dataprof-db/tests/structured_column_decoding.rs +++ b/crates/dataprof-db/tests/structured_column_decoding.rs @@ -93,6 +93,39 @@ fn profile_wrongs(report: &ProfileReport, expected: &[(&str, DataType)]) -> Vec< wrong } +fn duplicate_rows(report: &ProfileReport) -> Option { + report + .quality + .as_ref() + .and_then(|quality| quality.metrics.uniqueness.as_ref()) + .map(|uniqueness| uniqueness.duplicate_rows) +} + +/// Rows of the column `name` whose value was a container, per path. +fn container_rows_wrongs( + paths: &[(&'static str, QueryColumns)], + name: &str, + expected: &[usize], +) -> Vec { + let mut wrong = Vec::new(); + for (path, columns) in paths { + let position = columns + .names() + .position(|column| column == name) + .expect("column in the result"); + let rows = columns + .container_rows() + .nth(position) + .expect("container rows"); + if rows != expected { + wrong.push(format!( + "{path} {name}: container rows {rows:?}, expected {expected:?}" + )); + } + } + wrong +} + #[cfg(feature = "postgres")] mod postgres { use super::*; @@ -100,14 +133,6 @@ mod postgres { const TABLE: &str = "SELECT * FROM structured_decoding_pg ORDER BY id"; - fn duplicate_rows(report: &ProfileReport) -> Option { - report - .quality - .as_ref() - .and_then(|quality| quality.metrics.uniqueness.as_ref()) - .map(|uniqueness| uniqueness.duplicate_rows) - } - /// Created once per process; the tests only read it. static FIXTURE: tokio::sync::OnceCell> = tokio::sync::OnceCell::const_new(); @@ -216,6 +241,30 @@ mod postgres { assert!(wrong.is_empty(), "{wrong:#?}"); } + /// A `jsonb` string holding an object's text renders the same text as the + /// object, so the row signature must tell them apart by the container + /// flag (#945). Rows 2 and 3 hold the object and are one duplicate; row 1 + /// holds the string and is not. + #[tokio::test] + async fn a_json_string_spelling_an_object_is_not_the_object() { + let Some(url) = url().await else { + return; + }; + let query = r#"SELECT id, j FROM (VALUES + (1, 1, '"{\"x\":1}"'::jsonb), + (2, 1, '{"x":1}'::jsonb), + (3, 1, '{"x":1}'::jsonb)) AS t(n, id, j) ORDER BY n"#; + + let mut wrong = container_rows_wrongs(&both_paths(&url, query).await, "j", &[1, 2]); + let duplicates = duplicate_rows(&profile(&url, query).await); + if duplicates != Some(1) { + wrong.push(format!( + "profile: {duplicates:?} duplicate rows, expected 1" + )); + } + assert!(wrong.is_empty(), "{wrong:#?}"); + } + /// PostgreSQL accepts numbers serde_json cannot hold. Such a document keeps /// the database's digits instead of falling back to null, and its keys are /// still sorted: the two rows differ only in key order, whitespace and @@ -388,4 +437,28 @@ mod mysql { ); assert!(wrong.is_empty(), "{wrong:#?}"); } + + /// A JSON string holding an object's text renders the same text as the + /// object, so the row signature must tell them apart by the container + /// flag (#945). Rows 2 and 3 hold the object and are one duplicate. + #[tokio::test] + async fn a_json_string_spelling_an_object_is_not_the_object() { + let Some(url) = url().await else { + return; + }; + // The streaming path refuses `UNION`, so the rows come from `VALUES`. + let query = r#"SELECT id, j FROM (VALUES + ROW(1, 1, CAST('"{\\"x\\":1}"' AS JSON)), + ROW(2, 1, CAST('{"x":1}' AS JSON)), + ROW(3, 1, CAST('{"x":1}' AS JSON))) AS t(n, id, j) ORDER BY n"#; + + let mut wrong = container_rows_wrongs(&both_paths(&url, query).await, "j", &[1, 2]); + let duplicates = duplicate_rows(&profile(&url, query).await); + if duplicates != Some(1) { + wrong.push(format!( + "profile: {duplicates:?} duplicate rows, expected 1" + )); + } + assert!(wrong.is_empty(), "{wrong:#?}"); + } } diff --git a/crates/dataprof-engines/src/progress_tracker.rs b/crates/dataprof-engines/src/progress_tracker.rs index 1b046294..3b1ca294 100644 --- a/crates/dataprof-engines/src/progress_tracker.rs +++ b/crates/dataprof-engines/src/progress_tracker.rs @@ -5,6 +5,8 @@ use dataprof_core::{ProgressEvent, ProgressSink}; /// Engine-internal tracker that rate-limits event emission. pub(crate) struct ProgressTracker { sink: ProgressSink, + #[cfg(feature = "async-streaming")] + terminal_permit: Option>, start_time: Instant, last_update: Instant, update_interval: Duration, @@ -16,8 +18,12 @@ pub(crate) struct ProgressTracker { impl ProgressTracker { pub(crate) fn new(sink: ProgressSink, interval: Duration) -> Self { let now = Instant::now(); + #[cfg(feature = "async-streaming")] + let terminal_permit = sink.reserve_terminal(); Self { sink, + #[cfg(feature = "async-streaming")] + terminal_permit, start_time: now, last_update: now, update_interval: interval, @@ -82,12 +88,18 @@ impl ProgressTracker { pub(crate) fn emit_finished(&mut self, truncated: bool) { let elapsed = Instant::now().duration_since(self.start_time); - self.emit(ProgressEvent::Finished { + let event = ProgressEvent::Finished { total_rows: self.total_rows, total_bytes: self.total_bytes, elapsed, truncated, - }); + }; + #[cfg(feature = "async-streaming")] + if let Some(permit) = self.terminal_permit.take() { + permit.send(event); + return; + } + self.emit(event); } fn emit(&self, event: ProgressEvent) { diff --git a/crates/dataprof-engines/src/streaming/async_reader.rs b/crates/dataprof-engines/src/streaming/async_reader.rs index 4d6635f8..e28e9fd0 100644 --- a/crates/dataprof-engines/src/streaming/async_reader.rs +++ b/crates/dataprof-engines/src/streaming/async_reader.rs @@ -334,6 +334,7 @@ impl AsyncStreamingProfiler { let ( _headers, mut column_stats, + mut progress_tracker, total_rows, sampled_rows, total_bytes, @@ -434,6 +435,7 @@ impl AsyncStreamingProfiler { execution = execution.with_unterminated_quote(ended_inside_quotes); } + let is_truncated = truncation_reason.is_some(); if let Some(reason) = truncation_reason { execution = execution.with_truncation(reason); } else if total_rows > 0 && sampled_rows < total_rows { @@ -444,6 +446,8 @@ impl AsyncStreamingProfiler { execution = execution.with_sampling(sampled_rows as f64 / total_rows as f64); } + progress_tracker.emit_finished(is_truncated); + Ok(ReportAssembler::new(data_source, execution) .columns(column_profiles) .with_quality_data(sample_columns) @@ -1055,6 +1059,7 @@ impl AsyncStreamingProfiler { ( Vec, StreamingColumnCollection, + ProgressTracker, usize, usize, u64, @@ -1170,7 +1175,7 @@ impl AsyncStreamingProfiler { sampler.offer((values, containers)); } else { count_containers(&headers, &containers, &mut container_counts); - column_stats.process_record(&headers, values); + column_stats.process_record_with_containers(&headers, values, &containers); sampled_rows += 1; } @@ -1273,15 +1278,14 @@ impl AsyncStreamingProfiler { // in here rather than row by row. for (values, containers) in sampler.take_sample() { count_containers(&headers, &containers, &mut container_counts); - column_stats.process_record(&headers, values); + column_stats.process_record_with_containers(&headers, values, &containers); sampled_rows += 1; } - progress_tracker.emit_finished(truncation_reason.is_some()); - Ok(( headers, column_stats, + progress_tracker, total_rows, sampled_rows, total_bytes, diff --git a/crates/dataprof-json/src/lib.rs b/crates/dataprof-json/src/lib.rs index e4c7f15e..006e9327 100644 --- a/crates/dataprof-json/src/lib.rs +++ b/crates/dataprof-json/src/lib.rs @@ -714,8 +714,14 @@ fn feed_json_object( obj.get(column).map(json_value_text).unwrap_or_default() }) .collect(); + let containers: Vec = known_columns + .iter() + .enumerate() + .filter(|(_, column)| matches!(obj.get(*column), Some(Value::Array(_) | Value::Object(_)))) + .map(|(position, _)| position) + .collect(); - column_stats.process_record(known_columns, values); + column_stats.process_record_with_containers(known_columns, values, &containers); } /// Analyze JSON/JSONL data from a buffered reader using streaming statistics. diff --git a/crates/dataprof-metrics/src/analysis/column.rs b/crates/dataprof-metrics/src/analysis/column.rs index 52255125..ef3a22fe 100644 --- a/crates/dataprof-metrics/src/analysis/column.rs +++ b/crates/dataprof-metrics/src/analysis/column.rs @@ -26,6 +26,22 @@ struct ColumnAnalysis { skip_unique_count: bool, } +/// The statistics a column carries: none when it holds no non-null value. +/// +/// Every statistic is an aggregate over values, and over no values it is +/// undefined, which the repo reports as absent, not zero. Each type's arm can +/// still produce one: a text column of nulls only reported lengths of 0, the +/// same as a column of real zero-length values (#983). Shared by this path and +/// `profile_builder::build_column_profile`, so every engine draws the line in +/// one place; a column with no rows is the case where both counts are 0. +pub fn stats_over_values(stats: ColumnStats, total_count: usize, null_count: usize) -> ColumnStats { + if null_count >= total_count { + ColumnStats::None + } else { + stats + } +} + /// Analyze a column with full profiling (includes pattern detection and unique counts) pub fn analyze_column(name: &str, data: &[String]) -> ColumnProfile { analyze_column_with_options( @@ -176,6 +192,7 @@ fn analyze_column_with_options( DataType::Nested => unreachable!("infer_type never answers Nested"), } }; + let stats = stats_over_values(stats, total_count, null_count); let patterns = if analysis.skip_patterns { None @@ -221,6 +238,28 @@ fn analyze_column_with_options( mod tests { use super::*; + /// The database connectors profile through this path. A column holding + /// only nulls and null tokens has no values, so no statistics, where it + /// used to report text lengths of 0 (#983); one value restores them. + #[test] + fn a_column_without_a_value_has_no_statistics() { + let nulls: Vec = ["", "NA", "null", " "].map(String::from).to_vec(); + let profile = analyze_column("empty", &nulls); + assert_eq!((profile.null_count, profile.total_count), (4, 4)); + assert!( + matches!(profile.stats, ColumnStats::None), + "{:?}", + profile.stats + ); + + let mut one = nulls.clone(); + one.push("ab".to_string()); + match analyze_column("one", &one).stats { + ColumnStats::Text(text) => assert_eq!((text.min_length, text.max_length), (2, 2)), + other => panic!("expected text statistics, got {other:?}"), + } + } + #[test] fn non_finite_numeric_column_has_no_statistics() { let data = ["Infinity", "-inf"].map(String::from); diff --git a/crates/dataprof-metrics/src/analysis/metrics/bounds.rs b/crates/dataprof-metrics/src/analysis/metrics/bounds.rs index b12e9404..667c6bec 100644 --- a/crates/dataprof-metrics/src/analysis/metrics/bounds.rs +++ b/crates/dataprof-metrics/src/analysis/metrics/bounds.rs @@ -121,7 +121,7 @@ pub(crate) fn score_bounds( timeliness, data, column_profiles, - &semantic_hints.positive_columns, + semantic_hints, temporal_columns, )?; @@ -420,10 +420,11 @@ fn collect_shares( timeliness: &TimelinessCalculator<'_>, data: &HashMap>, column_profiles: &[ColumnProfile], - positive_columns: &[String], + semantic_hints: &SemanticHints, temporal_columns: &[String], ) -> Result>>, DataProfilerError> { let wants = |dimension: QualityDimension| wanted.contains(&dimension); + let positive_columns = &semantic_hints.positive_columns; let accuracy = AccuracyCalculator::new(thresholds); let (mut types, mut formats, mut encodings) = (Vec::new(), Vec::new(), Vec::new()); let (mut outliers, mut violations) = (Vec::new(), Vec::new()); @@ -485,7 +486,7 @@ fn collect_shares( )); } if wants(QualityDimension::Timeliness) && temporal_columns.contains(&profile.name) { - let metrics = timeliness.calculate(&single, temporal_columns, column_profiles)?; + let metrics = timeliness.calculate(&single, temporal_columns, &[], column_profiles)?; stale.push(counts(metrics.stale_dates, metrics.valid_dates, 1)); // A value is either a date, which may be in the future, or invalid. date_failures.push(counts( @@ -510,7 +511,13 @@ fn collect_shares( let mut timeliness_shares = Some(vec![stale, date_failures]); if wants(QualityDimension::Timeliness) { - match temporal_pairs(timeliness, data, column_profiles, temporal_columns)? { + match temporal_pairs( + timeliness, + data, + column_profiles, + temporal_columns, + &semantic_hints.temporal_pairs, + )? { Pairs::None => {} Pairs::Aligned(pairs) => { if let Some(shares) = timeliness_shares.as_mut() { @@ -568,8 +575,9 @@ fn temporal_pairs( data: &HashMap>, column_profiles: &[ColumnProfile], temporal_columns: &[String], + declared_pairs: &[(String, String)], ) -> Result { - let metrics = calculator.calculate(data, temporal_columns, column_profiles)?; + let metrics = calculator.calculate(data, temporal_columns, declared_pairs, column_profiles)?; if metrics.temporal_pairs_checked == 0 { return Ok(Pairs::None); } diff --git a/crates/dataprof-metrics/src/analysis/metrics/hint_binding.rs b/crates/dataprof-metrics/src/analysis/metrics/hint_binding.rs index 5336eb42..c0128361 100644 --- a/crates/dataprof-metrics/src/analysis/metrics/hint_binding.rs +++ b/crates/dataprof-metrics/src/analysis/metrics/hint_binding.rs @@ -15,8 +15,8 @@ use dataprof_core::{SemanticHintBinding, SemanticHintKind, SemanticHints}; use super::utils::extract_year; use crate::analysis::inference::is_null_like_token; -/// Compute binding evidence for the value-driven hints (`positive_columns` and -/// `temporal_columns`) over `data`. +/// Compute binding evidence for the value-driven hints (`positive_columns`, +/// `temporal_columns` and the columns of `temporal_pairs`) over `data`. /// /// `exact` states whether `data` covers every row (`true`) or is a sample /// (`false`); it is copied onto each binding so a proven-inert hint can be told @@ -44,17 +44,19 @@ pub fn compute_value_hint_bindings( } } - for column in &hints.temporal_columns { + let temporal = hints + .temporal_columns + .iter() + .map(|column| (SemanticHintKind::Temporal, column.as_str())); + let pairs = hints + .temporal_pair_columns() + .into_iter() + .map(|column| (SemanticHintKind::TemporalPair, column)); + for (kind, column) in temporal.chain(pairs) { if let Some(values) = data.get(column) { // Mirror timeliness.rs: a value counts as a date if a year parses. - let (checked, matched) = count_matches(values, SemanticHintKind::Temporal); - bindings.push(binding( - column, - SemanticHintKind::Temporal, - checked, - matched, - exact, - )); + let (checked, matched) = count_matches(values, kind); + bindings.push(binding(column, kind, checked, matched, exact)); } } @@ -112,7 +114,9 @@ pub fn value_matches_hint(value: &str, kind: SemanticHintKind) -> bool { .parse::() .is_ok_and(|number| number.is_finite()), // Mirror timeliness.rs: it extracts a year from the original cell. - SemanticHintKind::Temporal => extract_year(value).is_some(), + SemanticHintKind::Temporal | SemanticHintKind::TemporalPair => { + extract_year(value).is_some() + } // Identifier binding is structural and is not value-driven. SemanticHintKind::Identifier => true, } diff --git a/crates/dataprof-metrics/src/analysis/metrics/mod.rs b/crates/dataprof-metrics/src/analysis/metrics/mod.rs index dcc4c03a..5d7f422b 100644 --- a/crates/dataprof-metrics/src/analysis/metrics/mod.rs +++ b/crates/dataprof-metrics/src/analysis/metrics/mod.rs @@ -147,14 +147,21 @@ impl MetricsCalculator { } } - /// Explicit temporal hints plus columns the profiler itself inferred as dates. - /// Hints come first; inferred columns are appended in stable profile order - /// and duplicates are removed. + /// Explicit temporal hints, then the columns of declared pairs, plus columns + /// the profiler itself inferred as dates. Hints come first; inferred columns + /// are appended in stable profile order and duplicates are removed. fn effective_temporal_columns( column_profiles: &[ColumnProfile], semantic_hints: &SemanticHints, ) -> Vec { let mut columns = semantic_hints.temporal_columns.clone(); + for (start, end) in &semantic_hints.temporal_pairs { + for name in [start, end] { + if !columns.contains(name) { + columns.push(name.clone()); + } + } + } for profile in column_profiles { if profile.data_type == DataType::Date && !columns.contains(&profile.name) { columns.push(profile.name.clone()); @@ -295,6 +302,7 @@ impl MetricsCalculator { let t = TimelinessCalculator::new(&self.thresholds).calculate( data, &temporal_columns, + &semantic_hints.temporal_pairs, column_profiles, )?; Some(TimelinessMetrics { @@ -613,7 +621,12 @@ impl MetricsCalculator { let timeliness = if Self::is_requested(requested, QualityDimension::Timeliness) { let t = if !data.is_empty() { - timeliness_calculator.calculate(data, &temporal_columns, column_profiles)? + timeliness_calculator.calculate( + data, + &temporal_columns, + &semantic_hints.temporal_pairs, + column_profiles, + )? } else { timeliness::TimelinessMetrics { future_dates_count: 0, diff --git a/crates/dataprof-metrics/src/analysis/metrics/timeliness.rs b/crates/dataprof-metrics/src/analysis/metrics/timeliness.rs index 3d0ea68e..4ca831f8 100644 --- a/crates/dataprof-metrics/src/analysis/metrics/timeliness.rs +++ b/crates/dataprof-metrics/src/analysis/metrics/timeliness.rs @@ -181,6 +181,7 @@ impl<'a> TimelinessCalculator<'a> { &self, data: &HashMap>, temporal_columns: &[String], + declared_pairs: &[(String, String)], column_profiles: &[ColumnProfile], ) -> Result { let summary = self.calculate_date_summary(data, temporal_columns); @@ -189,7 +190,12 @@ impl<'a> TimelinessCalculator<'a> { } else { (summary.stale_dates as f64 / summary.valid_dates as f64) * 100.0 }; - let ordering = Self::count_temporal_violations(data, temporal_columns, column_profiles)?; + let ordering = Self::count_temporal_violations( + data, + temporal_columns, + declared_pairs, + column_profiles, + )?; Ok(TimelinessMetrics { future_dates_count: summary.future_dates, @@ -282,9 +288,15 @@ impl<'a> TimelinessCalculator<'a> { /// left unpaired rather than guessed. Names that share nothing still pair /// when they are the only start and the only end, as /// `created_at`/`last_updated_at` are. + /// + /// Declared pairs come first and are compared as given (#942). A column + /// named in one takes no part in the pairing by name, which pairs the + /// rest: declaring `trip_start`/`trip_end` leaves `trip_start_date` free + /// to pair by its own name. fn column_pairs<'d>( data: &HashMap>, temporal_columns: &'d [String], + declared_pairs: &'d [(String, String)], ) -> Vec<(&'d str, &'d str)> { let temporal_pairs = [ ("start_date", "end_date"), @@ -302,6 +314,20 @@ impl<'a> TimelinessCalculator<'a> { let mut evaluated: HashSet<(&str, &str)> = HashSet::new(); let mut pairs = Vec::new(); + let declared: HashSet<&str> = declared_pairs + .iter() + .flat_map(|(start, end)| [start.as_str(), end.as_str()]) + .collect(); + for (start_name, end_name) in declared_pairs { + if start_name != end_name + && data.contains_key(start_name) + && data.contains_key(end_name) + && evaluated.insert((start_name, end_name)) + { + pairs.push((start_name.as_str(), end_name.as_str())); + } + } + for (start_col, end_col) in &temporal_pairs { // Walk `temporal_columns`, not `data`: the HashMap's iteration order // is randomized. A name the caller listed but the data does not @@ -309,7 +335,7 @@ impl<'a> TimelinessCalculator<'a> { let holders = |role: &str| -> Vec<(&'d str, Vec)> { temporal_columns .iter() - .filter(|name| data.contains_key(*name)) + .filter(|name| data.contains_key(*name) && !declared.contains(name.as_str())) .filter_map(|name| Some((name.as_str(), words_beside_role(name, role)?))) .collect() }; @@ -381,6 +407,7 @@ impl<'a> TimelinessCalculator<'a> { fn count_temporal_violations( data: &HashMap>, temporal_columns: &[String], + declared_pairs: &[(String, String)], column_profiles: &[ColumnProfile], ) -> Result { let mut violations = 0; @@ -388,7 +415,7 @@ impl<'a> TimelinessCalculator<'a> { let mut compared = Vec::new(); let profile = |name: &str| column_profiles.iter().find(|profile| profile.name == name); - for (start_name, end_name) in Self::column_pairs(data, temporal_columns) { + for (start_name, end_name) in Self::column_pairs(data, temporal_columns, declared_pairs) { let (Some(start_values), Some(end_values)) = (data.get(start_name), data.get(end_name)) else { continue; @@ -474,7 +501,7 @@ mod tests { data: &HashMap>, temporal_columns: &[String], ) -> Result { - self.calculate(data, temporal_columns, &whole(data)) + self.calculate(data, temporal_columns, &[], &whole(data)) } } @@ -757,6 +784,126 @@ mod tests { } } + /// Metrics with `declared` pairs, over `temporal_columns` as the metrics + /// pass them: hinted columns, then the columns of the declared pairs. + fn metrics_with_pairs( + data: &HashMap>, + temporal_columns: &[&str], + declared: &[(&str, &str)], + ) -> TimelinessMetrics { + let named: Vec = temporal_columns + .iter() + .map(|name| name.to_string()) + .collect(); + let declared: Vec<(String, String)> = declared + .iter() + .map(|(start, end)| (start.to_string(), end.to_string())) + .collect(); + TimelinessCalculator::with_reference_date(&config(), reference()) + .calculate(data, &named, &declared, &whole(data)) + .expect("timeliness metrics") + } + + fn sorted(pairs: &[(String, String)]) -> Vec<(&str, &str)> { + let mut pairs: Vec<(&str, &str)> = pairs + .iter() + .map(|(start, end)| (start.as_str(), end.as_str())) + .collect(); + pairs.sort(); + pairs + } + + #[test] + fn a_declared_pair_is_compared_whatever_its_names() { + // #942: no role word fits these names, so they were never compared. + let data = HashMap::from([ + ("valid_from".to_string(), vec!["2024-01-05".to_string()]), + ("valid_until".to_string(), vec!["2024-01-01".to_string()]), + ("checkin".to_string(), vec!["2024-03-01".to_string()]), + ("checkout".to_string(), vec!["2024-03-04".to_string()]), + ]); + let columns = ["valid_from", "valid_until", "checkin", "checkout"]; + + let metrics = metrics_with_pairs(&data, &columns, &[]); + assert!(metrics.compared_pairs.is_empty()); + + let declared = [("valid_from", "valid_until"), ("checkin", "checkout")]; + let metrics = metrics_with_pairs(&data, &columns, &declared); + assert_eq!(metrics.temporal_pairs_checked, 2); + assert_eq!(metrics.temporal_violations, 1); + assert_eq!( + sorted(&metrics.compared_pairs), + [("checkin", "checkout"), ("valid_from", "valid_until")] + ); + } + + #[test] + fn a_declared_pair_settles_names_that_are_ambiguous() { + // The #913 case, which pairs nothing by name. + let data = HashMap::from([ + ("primary_start".to_string(), vec!["2024-01-01".to_string()]), + ( + "secondary_start".to_string(), + vec!["2024-01-03".to_string()], + ), + ("end".to_string(), vec!["2024-01-02".to_string()]), + ]); + let metrics = metrics_with_pairs( + &data, + &["primary_start", "secondary_start", "end"], + &[("secondary_start", "end")], + ); + + assert_eq!(metrics.temporal_pairs_checked, 1); + assert_eq!(metrics.temporal_violations, 1); + assert_eq!( + sorted(&metrics.compared_pairs), + [("secondary_start", "end")] + ); + } + + #[test] + fn columns_outside_the_declared_pairs_still_pair_by_name() { + let data = HashMap::from([ + ("valid_from".to_string(), vec!["2024-01-01".to_string()]), + ("valid_until".to_string(), vec!["2024-01-05".to_string()]), + ("billing_start".to_string(), vec!["2024-06-30".to_string()]), + ("billing_end".to_string(), vec!["2024-06-01".to_string()]), + ]); + let metrics = metrics_with_pairs( + &data, + &["billing_start", "billing_end", "valid_from", "valid_until"], + &[("valid_from", "valid_until")], + ); + + assert_eq!(metrics.temporal_pairs_checked, 2); + assert_eq!(metrics.temporal_violations, 1); + assert_eq!( + sorted(&metrics.compared_pairs), + [ + ("billing_start", "billing_end"), + ("valid_from", "valid_until") + ] + ); + } + + #[test] + fn a_column_in_a_declared_pair_is_not_paired_again_by_name() { + // `start` and `end` would pair by name; declaring `start` with + // `finish` says which end it has. + let data = HashMap::from([ + ("start".to_string(), vec!["2024-01-03".to_string()]), + ("end".to_string(), vec!["2024-01-01".to_string()]), + ("finish".to_string(), vec!["2024-01-05".to_string()]), + ]); + let metrics = + metrics_with_pairs(&data, &["start", "end", "finish"], &[("start", "finish")]); + + assert_eq!(metrics.temporal_pairs_checked, 1); + assert_eq!(metrics.temporal_violations, 0); + assert_eq!(sorted(&metrics.compared_pairs), [("start", "finish")]); + } + // ---------------------------------------------------------------- #378 // Deterministic coverage for the timeliness calculator. Every case pins // "now" through `with_reference_date`, so no assertion here changes result diff --git a/crates/dataprof-parquet/src/record_batch_analyzer.rs b/crates/dataprof-parquet/src/record_batch_analyzer.rs index ab50464c..89065da5 100644 --- a/crates/dataprof-parquet/src/record_batch_analyzer.rs +++ b/crates/dataprof-parquet/src/record_batch_analyzer.rs @@ -506,7 +506,11 @@ impl RecordBatchAnalyzer { for field in schema.fields() { let column_name = field.name().to_string(); let positive_hint = self.semantic_hints.is_positive_column(&column_name); - let temporal_hint = self.semantic_hints.is_temporal_column(&column_name); + let temporal_hint = self.semantic_hints.is_temporal_column(&column_name) + || self + .semantic_hints + .temporal_pair_columns() + .contains(&column_name.as_str()); if let Entry::Vacant(entry) = self.column_analyzers.entry(column_name.clone()) { self.column_order.push(column_name); entry.insert(ColumnAnalyzer::with_value_hints( @@ -636,7 +640,16 @@ impl RecordBatchAnalyzer { .get(column) .map(|analyzer| analyzer.hint_binding(column, SemanticHintKind::Temporal)) }); - positive.chain(temporal).collect() + let pairs = self + .semantic_hints + .temporal_pair_columns() + .into_iter() + .filter_map(|column| { + self.column_analyzers + .get(column) + .map(|analyzer| analyzer.hint_binding(column, SemanticHintKind::TemporalPair)) + }); + positive.chain(temporal).chain(pairs).collect() } } @@ -755,7 +768,9 @@ impl ColumnAnalyzer { fn hint_binding(&self, column: &str, kind: SemanticHintKind) -> SemanticHintBinding { let matched_values = match kind { SemanticHintKind::Positive => self.positive_matched_values, - SemanticHintKind::Temporal => self.temporal_matched_values, + SemanticHintKind::Temporal | SemanticHintKind::TemporalPair => { + self.temporal_matched_values + } SemanticHintKind::Identifier => self.hint_checked_values, }; SemanticHintBinding { diff --git a/crates/dataprof-python/src/arrow_export.rs b/crates/dataprof-python/src/arrow_export.rs index 03ea6374..aacc7f2c 100644 --- a/crates/dataprof-python/src/arrow_export.rs +++ b/crates/dataprof-python/src/arrow_export.rs @@ -954,10 +954,11 @@ fn convert_pandas_to_batches(py: Python<'_>, df: &Bound<'_, PyAny>) -> PyResult< ) })?; - // Convert pandas to pyarrow Table - let pa_table = pyarrow - .getattr("Table")? - .call_method1("from_pandas", (df,))?; + // Convert pandas to pyarrow Table, falling back to string arrays for mixed object columns (#976) + let inputs_mod = py.import("dataprof._inputs")?; + let pa_table = inputs_mod + .getattr("_pandas_to_pyarrow_table")? + .call1((df, &pyarrow))?; import_batches_from_pyarrow(py, &pa_table) } diff --git a/crates/dataprof-python/src/columns.rs b/crates/dataprof-python/src/columns.rs index aa3a195c..750a0dbc 100644 --- a/crates/dataprof-python/src/columns.rs +++ b/crates/dataprof-python/src/columns.rs @@ -172,9 +172,14 @@ pub fn profile_columns( for row_index in 0..num_rows { let mut row_signature = RowSignature::default(); let mut row_has_null = false; - for (_, cells, _) in &columns { + for (_, cells, containers) in &columns { let value = cells[row_index].as_deref().unwrap_or(""); - row_signature.push_field(value); + // Validated above as ascending, so a binary search holds. + if containers.binary_search(&row_index).is_ok() { + row_signature.push_container_field(value); + } else { + row_signature.push_field(value); + } // A missing cell renders empty, which `is_null_like_token` // already treats as null — the same rule that produces // `null_count` below. diff --git a/crates/dataprof-python/src/config.rs b/crates/dataprof-python/src/config.rs index 536f289a..031fd45b 100644 --- a/crates/dataprof-python/src/config.rs +++ b/crates/dataprof-python/src/config.rs @@ -38,6 +38,7 @@ pub struct PyProfilerConfig { pub(crate) positive_columns: Vec, pub(crate) identifier_columns: Vec, pub(crate) temporal_columns: Vec, + pub(crate) temporal_pairs: Vec<(String, String)>, } #[pymethods] @@ -63,6 +64,7 @@ impl PyProfilerConfig { identifier_columns = None, temporal_columns = None, columns = None, + temporal_pairs = None, ))] #[allow(clippy::too_many_arguments)] fn new( @@ -85,6 +87,7 @@ impl PyProfilerConfig { identifier_columns: Option>, temporal_columns: Option>, columns: Option>, + temporal_pairs: Option>>, ) -> PyResult { let engine = parse_engine(engine)?; let format_override = format.map(parse_format).transpose()?; @@ -135,6 +138,20 @@ impl PyProfilerConfig { // return a plausible, emptier report with nothing saying why. let locale = Locale::parse_optional(locale.as_deref()).map_err(PyValueError::new_err)?; + // Taken as sequences, so a pair read from JSON as a list works like a + // tuple; anything but two names is refused rather than cut to fit. + let temporal_pairs = temporal_pairs + .unwrap_or_default() + .into_iter() + .map(|pair| match <[String; 2]>::try_from(pair) { + Ok([start, end]) => Ok((start, end)), + Err(pair) => Err(PyValueError::new_err(format!( + "temporal_pairs entries must be (start, end) column pairs, got {} name(s)", + pair.len() + ))), + }) + .collect::>>()?; + // Parse metrics strings into MetricPack enums let metric_packs = metrics .map(|packs| { @@ -165,6 +182,7 @@ impl PyProfilerConfig { positive_columns: positive_columns.unwrap_or_default(), identifier_columns: identifier_columns.unwrap_or_default(), temporal_columns: temporal_columns.unwrap_or_default(), + temporal_pairs, }) } @@ -253,6 +271,12 @@ impl PyProfilerConfig { self.temporal_columns.clone() } + /// Start/end column pairs timeliness compares as given + #[getter] + fn temporal_pairs(&self) -> Vec<(String, String)> { + self.temporal_pairs.clone() + } + fn __repr__(&self) -> String { format!( "ProfilerConfig(engine='{}', chunk_size={}, memory_limit_mb={}, format={}, max_rows={})", @@ -330,6 +354,9 @@ impl PyProfilerConfig { if !self.temporal_columns.is_empty() { profiler = profiler.temporal_columns(self.temporal_columns.clone()); } + if !self.temporal_pairs.is_empty() { + profiler = profiler.temporal_pairs(self.temporal_pairs.clone()); + } profiler } @@ -340,6 +367,7 @@ impl PyProfilerConfig { self.identifier_columns.clone(), ) .with_temporal_columns(self.temporal_columns.clone()) + .with_temporal_pairs(self.temporal_pairs.clone()) } /// The analysis selection this config asks for. diff --git a/crates/dataprof-python/src/lib.rs b/crates/dataprof-python/src/lib.rs index 1e71bb25..bff25eac 100644 --- a/crates/dataprof-python/src/lib.rs +++ b/crates/dataprof-python/src/lib.rs @@ -32,7 +32,7 @@ pub use partial::{ pub use progress::PyProgressEvent; pub use sampling::PySamplingStrategy; pub use stop_condition::PyStopCondition; -pub use types::{PyColumnProfile, PyDataQualityMetrics, PyProfileReport}; +pub use types::{PyColumnProfile, PyDataQualityMetrics, PyProfileReport, report_delta_json}; use pyo3::prelude::*; use pyo3::types::{PyDict, PyModule}; @@ -97,6 +97,7 @@ pub fn dataprof(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_function(wrap_pyfunction!(analyze_file, m)?)?; m.add_function(wrap_pyfunction!(profile_parquet_bytes, m)?)?; m.add_function(wrap_pyfunction!(list_patterns, m)?)?; + m.add_function(wrap_pyfunction!(report_delta_json, m)?)?; // DataFrame/Arrow profiling m.add_function(wrap_pyfunction!(profile_dataframe, m)?)?; diff --git a/crates/dataprof-python/src/types.rs b/crates/dataprof-python/src/types.rs index d816efe5..d79a3458 100644 --- a/crates/dataprof-python/src/types.rs +++ b/crates/dataprof-python/src/types.rs @@ -1223,6 +1223,7 @@ impl PyProfileReport { SemanticHintKind::Positive => "positive", SemanticHintKind::Identifier => "identifier", SemanticHintKind::Temporal => "temporal", + SemanticHintKind::TemporalPair => "temporal_pair", }, )?; dict.set_item("checked_values", b.checked_values)?; @@ -1329,3 +1330,34 @@ impl PyProfileReport { ) } } + +/// One side of `report_delta_json`: a native report, or the flat summary of +/// a report loaded from one, as JSON. +#[derive(FromPyObject)] +pub enum DeltaInput<'py> { + Native(PyRef<'py, PyProfileReport>), + Summary(String), +} + +impl DeltaInput<'_> { + fn side(&self) -> PyResult { + match self { + Self::Native(report) => Ok(dataprof::ReportSide::from_report(&report.inner)), + Self::Summary(text) => dataprof::ReportSide::from_summary_json(text).map_err(|e| { + pyo3::exceptions::PyValueError::new_err(format!( + "compare(): the flat summary cannot be compared: {e}" + )) + }), + } + } +} + +/// Compare two reports with the runtime delta and return its document as +/// JSON. `ProfileReport.compare()` converts it into the dict it returns. +#[pyfunction] +pub fn report_delta_json(a: DeltaInput<'_>, b: DeltaInput<'_>) -> PyResult { + let delta = dataprof::ReportDelta::between_sides(&a.side()?, &b.side()?); + serde_json::to_string(&delta).map_err(|e| { + pyo3::exceptions::PyRuntimeError::new_err(format!("JSON serialization failed: {e}")) + }) +} diff --git a/crates/dataprof-runtime/src/hint_binding.rs b/crates/dataprof-runtime/src/hint_binding.rs index 3c72d911..87d2a45e 100644 --- a/crates/dataprof-runtime/src/hint_binding.rs +++ b/crates/dataprof-runtime/src/hint_binding.rs @@ -36,8 +36,20 @@ impl ValueHintBindingAccumulator { exact: true, }, }); + let pairs = hints + .temporal_pair_columns() + .into_iter() + .map(|column| BindingCounter { + binding: SemanticHintBinding { + column: column.to_string(), + kind: SemanticHintKind::TemporalPair, + checked_values: 0, + matched_values: 0, + exact: true, + }, + }); Self { - counters: positive.chain(temporal).collect(), + counters: positive.chain(temporal).chain(pairs).collect(), } } diff --git a/crates/dataprof-runtime/src/lib.rs b/crates/dataprof-runtime/src/lib.rs index d809b1c3..d4f373dd 100644 --- a/crates/dataprof-runtime/src/lib.rs +++ b/crates/dataprof-runtime/src/lib.rs @@ -8,6 +8,7 @@ pub mod profile_builder; pub mod profile_report; pub mod quality_gate; pub mod report_assembler; +pub mod report_delta; pub mod streaming_stats; #[cfg(feature = "async-streaming")] @@ -37,6 +38,10 @@ pub use quality_gate::{ MetricValue, NotEvaluated, PolicyError, PolicyScope, QualityPolicy, RequiredMetric, Verdict, }; pub use report_assembler::ReportAssembler; +pub use report_delta::{ + ColumnChange, DimensionChange, ReportDelta, ReportReference, ReportSide, SchemaChange, + ScoreChange, SemanticsComparison, Sides, +}; pub use streaming_stats::{ RowCompletenessTracker, RowSignature, RowUniquenessTracker, StreamReservoirSampler, StreamingColumnCollection, StreamingStatistics, TextLengthStats, diff --git a/crates/dataprof-runtime/src/profile_builder.rs b/crates/dataprof-runtime/src/profile_builder.rs index e10f77dd..ec69ac67 100644 --- a/crates/dataprof-runtime/src/profile_builder.rs +++ b/crates/dataprof-runtime/src/profile_builder.rs @@ -12,6 +12,7 @@ use dataprof_core::{ TextStats, }; use dataprof_metrics::{ + analysis::column::stats_over_values, analysis::inference::{ classify_lexical_forms, count_locale_numbers, holds_digit_codes, is_inferred_date_token, is_integer_token, is_null_like_token, parse_strict_boolean_token, @@ -215,26 +216,18 @@ pub fn build_column_profile(input: ColumnProfileInput<'_>) -> ColumnProfile { } }; - // A column with no rows has counts, but no statistics: every aggregate over - // zero values is undefined, and the repo's convention for undefined is - // absent, not zero. Text and date columns already landed on - // `ColumnStats::None` here through their empty-sample arms; numeric and - // boolean did not, and reported a min, max, mean, std_dev and variance of - // 0.0, and a true_ratio of 0.0 -- plausible numbers describing rows that do - // not exist, which is the failure class AGENTS.md singles out. 0.0 is an - // ordinary value for a real numeric column, so nothing about that output - // looked wrong. - // - // Numeric and boolean columns also suppress statistics above when rows - // exist but no values parsed. This guard covers zero rows for every type. + // A column with no non-null value has counts, but no statistics: every + // aggregate over zero values is undefined, and the repo's convention for + // undefined is absent, not zero. Without this guard, a column with no rows + // reported a numeric min, max, mean, std_dev and variance of 0.0 and a + // true_ratio of 0.0, and a text column of nulls reported lengths of 0 + // (#983): plausible numbers describing values that do not exist, which is + // the failure class AGENTS.md singles out. 0.0 and 0 are ordinary values + // for a real column, so nothing about that output looked wrong. // // The counts above are untouched. "0 of 0 values were invalid" is a fact // about a column that was analyzed, not a statistic over nothing. - let stats = if input.total_count == 0 { - ColumnStats::None - } else { - stats - }; + let stats = stats_over_values(stats, input.total_count, input.null_count); let patterns = if input.skip_patterns { None diff --git a/crates/dataprof-runtime/src/profile_report.rs b/crates/dataprof-runtime/src/profile_report.rs index 66a1af53..54d9437b 100644 --- a/crates/dataprof-runtime/src/profile_report.rs +++ b/crates/dataprof-runtime/src/profile_report.rs @@ -144,6 +144,23 @@ impl MetricSemantics { shape_moments: Some(ShapeMomentEstimator::AdjustedSample), } } + + /// Whether every definition this build knows is recorded. A comparison + /// can only vouch for definitions both reports declare, so an unrecorded + /// one leaves it unknown. Destructured, so a new field fails to compile + /// here until it is counted. + pub(crate) fn is_fully_recorded(&self) -> bool { + let Self { + text_length_unit, + null_tokens, + pattern_match_count, + shape_moments, + } = self; + text_length_unit.is_some() + && null_tokens.is_some() + && pattern_match_count.is_some() + && shape_moments.is_some() + } } /// Complete profiling report for a data source. diff --git a/crates/dataprof-runtime/src/quality_gate.rs b/crates/dataprof-runtime/src/quality_gate.rs index 7673f211..771d6a8b 100644 --- a/crates/dataprof-runtime/src/quality_gate.rs +++ b/crates/dataprof-runtime/src/quality_gate.rs @@ -169,10 +169,11 @@ pub enum EvidenceGap { /// Every row was scanned, but this metric was computed from a retained /// sample of them. QualitySampled, - /// The report does not record how its quality numbers were obtained, so - /// whether they cover every scanned row is unknown. Only reachable for a - /// report read back from a document written before dataprof recorded it; - /// a profiling run always records it. Unknown coverage is not full + /// The report does not record how its numbers were obtained: which quality + /// components were sampled, or, in a flat summary from an older release, + /// whether the scan read the whole source. Only reachable for a report + /// read back from a document written before dataprof recorded it; a + /// profiling run always records it. Unknown coverage is not full /// coverage, so a full-source requirement is left unevaluated. CoverageUnrecorded, } @@ -221,7 +222,7 @@ impl Evidence { /// The weaker of two evidence statements: a number is only as complete as /// the least complete input behind it. The left-hand gap wins a tie, so /// the scan's gap is named ahead of the quality sample's. - fn and(self, other: Self) -> Self { + pub(crate) fn and(self, other: Self) -> Self { match self { Self::Complete => other, incomplete => incomplete, @@ -1172,12 +1173,22 @@ fn null_percentage(profile: &ColumnProfile) -> Option { /// /// Truncation is named ahead of sampling and sampling ahead of skipped /// records, so the gap reported is the largest one. -fn scan_evidence(execution: &ExecutionMetadata) -> Evidence { - let reason = if !execution.source_exhausted || execution.truncation_reason.is_some() { +pub(crate) fn scan_evidence(execution: &ExecutionMetadata) -> Evidence { + scan_coverage( + !execution.source_exhausted || execution.truncation_reason.is_some(), + execution.sampling_applied, + execution.error_count > 0, + ) +} + +/// [`scan_evidence`] over the three recorded facts it reads, for a report +/// delta reading them from a flat summary. +pub(crate) fn scan_coverage(truncated: bool, sampled: bool, records_skipped: bool) -> Evidence { + let reason = if truncated { Some(EvidenceGap::Truncated) - } else if execution.sampling_applied { + } else if sampled { Some(EvidenceGap::Sampled) - } else if execution.error_count > 0 { + } else if records_skipped { Some(EvidenceGap::RecordsSkipped) } else { None @@ -1191,7 +1202,7 @@ fn scan_evidence(execution: &ExecutionMetadata) -> Evidence { /// Which part of the assessment a check reads, and so whose provenance /// decides whether its number covers every scanned row. #[derive(Debug, Clone, Copy)] -enum Provenance { +pub(crate) enum Provenance { /// The aggregate over every dimension. Overall, /// One dimension, through whichever components it is built from. @@ -1205,7 +1216,7 @@ enum Provenance { /// Uniqueness is the one dimension with two, and they can differ in /// provenance: a full-stream row tracker counts duplicates over every row /// while the key scan reads the retained sample. -fn dimension_components(dimension: QualityDimension) -> &'static [&'static str] { +pub(crate) fn dimension_components(dimension: QualityDimension) -> &'static [&'static str] { match dimension { QualityDimension::Completeness => &["completeness"], QualityDimension::Consistency => &["consistency"], @@ -1228,7 +1239,23 @@ fn quality_evidence(quality: Option<&QualityAssessment>, provenance: Provenance) // absence itself, and there is no number for evidence to describe. return Evidence::Complete; }; - let Some(sampled) = quality.sampled_dimensions() else { + sampled_component_evidence( + quality.sampled_dimensions().as_deref(), + &quality.metrics.assessed_dimensions(), + provenance, + ) +} + +/// [`quality_evidence`] over the recorded facts it reads: the sampled +/// component labels (`None` when the report did not record them) and the +/// dimensions the overall score weighs. A report delta reads the same facts +/// from a flat summary, which has no [`QualityAssessment`] behind it. +pub(crate) fn sampled_component_evidence( + sampled: Option<&[String]>, + assessed: &[QualityDimension], + provenance: Provenance, +) -> Evidence { + let Some(sampled) = sampled else { return Evidence::Incomplete { reason: EvidenceGap::CoverageUnrecorded, }; @@ -1239,10 +1266,9 @@ fn quality_evidence(quality: Option<&QualityAssessment>, provenance: Provenance) // dimensions, so a sampled dimension the weights exclude does not // reach it. Reporting the aggregate as sampled because of one would // withhold a verdict the number does not depend on. - Provenance::Overall => quality - .metrics - .assessed_dimensions() - .into_iter() + Provenance::Overall => assessed + .iter() + .copied() .flat_map(dimension_components) .any(|component| contains(component)), Provenance::Dimension(dimension) => dimension_components(dimension) diff --git a/crates/dataprof-runtime/src/report_delta.rs b/crates/dataprof-runtime/src/report_delta.rs new file mode 100644 index 00000000..8958d121 --- /dev/null +++ b/crates/dataprof-runtime/src/report_delta.rs @@ -0,0 +1,632 @@ +//! What changed between two reports. +//! +//! A pipeline that keeps a baseline report asks the same questions of every +//! new one: did quality move, did a column appear or vanish, did a column's +//! null rate change. [`ReportDelta`] answers them in one typed value, so the +//! answer does not depend on which language computed it, and so a policy over +//! drift can be written against a contract rather than a dictionary. +//! +//! # The contract +//! +//! The two reports are `a` and `b`, compared in that order. The serialized +//! shape is the one `ProfileReport.compare()` returns in Python, and it is +//! stable: later releases may add keys, never remove or rename one. +//! +//! - **Signs.** Every change is `b - a`. A positive `abs` on a score is an +//! improvement, because a score is higher when quality is better. A positive +//! `null_pct_delta` is a degradation, because it is more nulls. +//! - **Precision.** Each side's number is taken at its serialized precision +//! (2 decimals on a 0-100 scale), and the change is rounded to it too, so a +//! report and its saved document compare alike. +//! - **No invented numbers.** A number one side does not carry, an +//! unassessed dimension, an empty column or an empty report, leaves the +//! change `None`. `rel_pct` is `None` as well when `a` is zero, where a +//! relative change is undefined. +//! - **Provenance.** Each number carries the [`Evidence`] behind it on both +//! sides, so an exact baseline compared with a sampled or truncated run says +//! so instead of reading a sampling difference as a change in the data. +//! - **Definitions.** `metric_semantics.comparable` is `Some(true)` only when +//! both reports record every definition this build knows, with the same +//! values. A difference in a measurement whose definition changed between +//! releases is not a difference in the data. +//! +//! Nothing here decides whether a change is acceptable; that is a policy's +//! job. A window-over-window comparison reuses the same vocabulary: `a` and +//! `b` name the two sides without saying which came first. +//! +//! # Example +//! +//! ```no_run +//! use dataprof_runtime::{ProfileReport, ReportDelta}; +//! +//! fn drift(baseline: &ProfileReport, current: &ProfileReport) { +//! let delta = ReportDelta::between(baseline, current); +//! if let Some(change) = delta.quality_score.abs { +//! println!("quality moved {change:+.2} points"); +//! } +//! for name in &delta.schema.removed { +//! println!("column {name} is gone"); +//! } +//! } +//! ``` + +use std::collections::{BTreeMap, HashSet}; + +use dataprof_core::QualityDimension; +use dataprof_core::serde_helpers::rounded_2; +use serde::ser::{SerializeMap, Serializer}; + +use crate::profile_report::{MetricSemantics, ProfileReport}; +use crate::quality_gate::{ + Evidence, EvidenceGap, Provenance, dimension_components, sampled_component_evidence, + scan_coverage, scan_evidence, +}; + +/// The same fact about each side of a comparison. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct Sides { + /// The first report. + pub a: T, + /// The second report. + pub b: T, +} + +/// Which report a side is, and how much of its source it read. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct ReportReference { + /// The report's identifier. `None` for a flat summary, which does not + /// record one. + pub id: Option, + /// When the report was generated. `None` for a flat summary. + pub timestamp: Option, + /// The source identifier: a path, a query, or an in-memory label. + pub source: String, + /// Rows the scan read. `None` for a flat summary that did not record it. + pub rows_processed: Option, + /// Whether the scan covered the whole source. + pub evidence: Evidence, +} + +/// One score on both sides and how it moved. +#[derive(Debug, Clone, PartialEq, serde::Serialize)] +#[non_exhaustive] +pub struct ScoreChange { + /// The score in `a`, or `None` when `a` did not assess it. + pub a: Option, + /// The score in `b`, or `None` when `b` did not assess it. + pub b: Option, + /// `b - a` in score points, when both sides have a score. + pub abs: Option, + /// `b - a` as a percentage of `a`, when both have a score and `a` is not + /// zero. + pub rel_pct: Option, + /// The evidence behind each side's score: the scan's coverage, and + /// whether the score was computed from a retained sample of the rows. + pub evidence: Sides, +} + +impl ScoreChange { + fn new(a: Option, b: Option, evidence: Sides) -> Self { + let (abs, rel_pct) = match (a, b) { + (Some(a), Some(b)) => ( + Some(rounded_2(b - a)), + (a != 0.0).then(|| rounded_2((b - a) / a.abs() * 100.0)), + ), + _ => (None, None), + }; + Self { + a, + b, + abs, + rel_pct, + evidence, + } + } +} + +/// One quality dimension's change. Serialized as an entry of the +/// `dimensions` object, keyed by the dimension's name. +#[derive(Debug, Clone, PartialEq)] +#[non_exhaustive] +pub struct DimensionChange { + /// The dimension. + pub dimension: QualityDimension, + /// Its score on both sides. + pub score: ScoreChange, +} + +/// One column's change. Serialized as an entry of the `columns` object, keyed +/// by the column's name. Each side's number covers what that side's scan +/// read; see [`ReportReference::evidence`]. +#[derive(Debug, Clone, PartialEq, serde::Serialize)] +#[non_exhaustive] +pub struct ColumnChange { + /// The column's name. + #[serde(skip)] + pub name: String, + /// Null percentage in `a`, or `None` when `a` has no such column or the + /// column holds no values. + pub null_pct_a: Option, + /// Null percentage in `b`, under the same rules. + pub null_pct_b: Option, + /// `null_pct_b - null_pct_a`, when both are present. Positive means more + /// nulls. + pub null_pct_delta: Option, +} + +/// Each side's recorded measurement definitions, and whether they agree. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct SemanticsComparison { + /// What `a` recorded, or `None` when it predates the record. + pub a: Option, + /// What `b` recorded. + pub b: Option, + /// `Some(true)` when both record every definition this build knows and + /// they match; `Some(false)` when they record different ones; `None` + /// when either side leaves a definition unrecorded, which includes every + /// report written before 0.12. + pub comparable: Option, +} + +/// Which columns each side has. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct SchemaChange { + /// In `b` only, in `b`'s order. + pub added: Vec, + /// In `a` only, in `a`'s order. + pub removed: Vec, + /// In both, in `a`'s order. + pub common: Vec, +} + +/// What changed between two reports. See the [module docs](self) for the +/// contract. +#[derive(Debug, Clone, PartialEq, serde::Serialize)] +#[non_exhaustive] +pub struct ReportDelta { + /// The overall quality score. + pub quality_score: ScoreChange, + /// Every quality dimension this build knows, in [`QualityDimension::all`] + /// order, assessed or not. + #[serde(serialize_with = "serialize_dimensions")] + pub dimensions: Vec, + /// The union of both sides' columns: `a`'s in its order, then the ones + /// only `b` has, in `b`'s order. + #[serde(serialize_with = "serialize_columns")] + pub columns: Vec, + /// Whether the two reports measured the same way. + pub metric_semantics: SemanticsComparison, + /// Columns added, removed and shared. + pub schema: SchemaChange, + /// Which two reports were compared. + pub reports: Sides, +} + +impl ReportDelta { + /// Compare two reports, `a` first. + pub fn between(a: &ProfileReport, b: &ProfileReport) -> Self { + Self::between_sides(&ReportSide::from_report(a), &ReportSide::from_report(b)) + } + + /// Compare two sides, either of which may come from a flat summary. + pub fn between_sides(a: &ReportSide, b: &ReportSide) -> Self { + let pair = |a: Evidence, b: Evidence| Sides { a, b }; + let quality_score = ScoreChange::new( + a.quality_score, + b.quality_score, + pair(a.overall_evidence, b.overall_evidence), + ); + let dimensions = a + .dimensions + .iter() + .zip(&b.dimensions) + .map(|(left, right)| { + debug_assert_eq!(left.dimension, right.dimension); + DimensionChange { + dimension: left.dimension, + score: ScoreChange::new( + left.score, + right.score, + pair(left.evidence, right.evidence), + ), + } + }) + .collect(); + + let a_names: HashSet<&str> = a.columns.iter().map(|(name, _)| name.as_str()).collect(); + let b_names: HashSet<&str> = b.columns.iter().map(|(name, _)| name.as_str()).collect(); + let a_nulls: BTreeMap<&str, Option> = a + .columns + .iter() + .map(|(name, pct)| (name.as_str(), *pct)) + .collect(); + let b_nulls: BTreeMap<&str, Option> = b + .columns + .iter() + .map(|(name, pct)| (name.as_str(), *pct)) + .collect(); + let added: Vec = b + .columns + .iter() + .filter(|(name, _)| !a_names.contains(name.as_str())) + .map(|(name, _)| name.clone()) + .collect(); + let columns = a + .columns + .iter() + .map(|(name, _)| name) + .chain(&added) + .map(|name| { + let null_pct_a = a_nulls.get(name.as_str()).copied().flatten(); + let null_pct_b = b_nulls.get(name.as_str()).copied().flatten(); + ColumnChange { + name: name.clone(), + null_pct_a, + null_pct_b, + null_pct_delta: null_pct_a.zip(null_pct_b).map(|(a, b)| rounded_2(b - a)), + } + }) + .collect(); + let (removed, common) = a + .columns + .iter() + .map(|(name, _)| name.clone()) + .partition(|name| !b_names.contains(name.as_str())); + + let comparable = match (&a.metric_semantics, &b.metric_semantics) { + (Some(left), Some(right)) if left.is_fully_recorded() && right.is_fully_recorded() => { + Some(left == right) + } + _ => None, + }; + + Self { + quality_score, + dimensions, + columns, + metric_semantics: SemanticsComparison { + a: a.metric_semantics.clone(), + b: b.metric_semantics.clone(), + comparable, + }, + schema: SchemaChange { + added, + removed, + common, + }, + reports: Sides { + a: a.reference.clone(), + b: b.reference.clone(), + }, + } + } +} + +fn serialize_dimensions( + dimensions: &[DimensionChange], + serializer: S, +) -> Result { + let mut map = serializer.serialize_map(Some(dimensions.len()))?; + for change in dimensions { + map.serialize_entry(&change.dimension.to_string(), &change.score)?; + } + map.end() +} + +fn serialize_columns( + columns: &[ColumnChange], + serializer: S, +) -> Result { + let mut map = serializer.serialize_map(Some(columns.len()))?; + for change in columns { + map.serialize_entry(&change.name, change)?; + } + map.end() +} + +/// What a comparison reads from one report. +/// +/// Built from a [`ProfileReport`], or from a flat summary (the document +/// [`ProfileReport::summary_json`] writes, and the one Python's `to_dict()` +/// and pre-0.12 `save()` produced). A flat summary never becomes a +/// `ProfileReport`, because it cannot acquire provenance it never recorded, +/// but it carries everything a comparison reads, and what it did not record +/// is read as unrecorded. +#[derive(Debug, Clone, PartialEq)] +#[non_exhaustive] +pub struct ReportSide { + reference: ReportReference, + quality_score: Option, + overall_evidence: Evidence, + dimensions: Vec, + columns: Vec<(String, Option)>, + metric_semantics: Option, +} + +#[derive(Debug, Clone, PartialEq)] +struct DimensionSide { + dimension: QualityDimension, + score: Option, + evidence: Evidence, +} + +/// One entry per column name, in order, holding the first profile of a +/// repeated name. Most input paths refuse duplicate names, but a report +/// rebuilt from a document can carry them, and the quality gate reads the +/// first profile too, so the two never disagree about which one a name means. +fn first_of_each_name( + columns: impl Iterator)>, +) -> Vec<(String, Option)> { + let mut seen = HashSet::new(); + columns + .filter(|(name, _)| seen.insert(name.clone())) + .collect() +} + +/// The overall score's coverage when `sampled` names a component this build +/// does not know: a dimension a later release added, whose weight is in the +/// stored overall score. Nothing rules out that it reaches the score, so the +/// score is not certified; `None` when every sampled label is known. +fn unknown_component_coverage(sampled: Option<&[String]>) -> Option { + let known = |label: &String| { + QualityDimension::all() + .into_iter() + .flat_map(dimension_components) + .any(|component| component == label) + }; + sampled + .is_some_and(|labels| !labels.iter().all(known)) + .then_some(Evidence::Incomplete { + reason: EvidenceGap::CoverageUnrecorded, + }) +} + +/// A number at its serialized precision; a non-finite one serializes as +/// null, so it is no number at all. +fn at_precision(value: Option) -> Option { + value.filter(|value| value.is_finite()).map(rounded_2) +} + +impl ReportSide { + /// Read a profiled or loaded report. + pub fn from_report(report: &ProfileReport) -> Self { + let scan = scan_evidence(&report.execution); + let quality = report.quality.as_ref(); + let scores = quality.map(|assessment| assessment.scores()); + let sampled = quality.map(|assessment| assessment.sampled_dimensions()); + let assessed = quality + .map(|assessment| assessment.metrics.assessed_dimensions()) + .unwrap_or_default(); + // No assessment means no score for evidence to describe; the scan's + // coverage is all there is to say. + let evidence = |provenance| match &sampled { + Some(sampled) => scan.and(sampled_component_evidence( + sampled.as_deref(), + &assessed, + provenance, + )), + None => scan, + }; + let dimensions = QualityDimension::all() + .into_iter() + .map(|dimension| DimensionSide { + dimension, + score: at_precision( + scores + .as_ref() + .and_then(|scores| scores.dimension_scores.get(&dimension.to_string())) + .copied() + .flatten(), + ), + evidence: evidence(Provenance::Dimension(dimension)), + }) + .collect(); + Self { + reference: ReportReference { + id: Some(report.id.clone()), + timestamp: Some(report.timestamp.clone()), + source: report.source_identifier(), + rows_processed: Some(report.execution.rows_processed), + evidence: scan, + }, + quality_score: at_precision(scores.as_ref().and_then(|scores| scores.overall_score)), + overall_evidence: unknown_component_coverage(sampled.clone().flatten().as_deref()) + .map_or_else(|| evidence(Provenance::Overall), |gap| scan.and(gap)), + dimensions, + columns: first_of_each_name(report.column_profiles.iter().map(|column| { + let pct = (column.total_count > 0) + .then(|| column.null_count as f64 / column.total_count as f64 * 100.0); + (column.name.clone(), at_precision(pct)) + })), + metric_semantics: report.metric_semantics.clone(), + } + } + + /// Read a flat summary document. + /// + /// A field an older release did not write reads as unrecorded: scan + /// coverage it does not state is `coverage_unrecorded`, never complete. + /// Fails when the document lacks `source` or `columns`, or holds a + /// field of the wrong type. + pub fn from_summary_json(text: &str) -> serde_json::Result { + let summary: Summary = serde_json::from_str(text)?; + let execution = &summary.execution; + let known_gap = scan_coverage( + execution.source_exhausted == Some(false) || execution.truncation_reason.is_some(), + execution.sampling_applied == Some(true), + execution.error_count.is_some_and(|count| count > 0), + ); + let recorded = execution.source_exhausted.is_some() + && execution.sampling_applied.is_some() + && execution.error_count.is_some(); + // A recorded gap is named; otherwise an unrecorded fact is a gap of + // its own, because unknown coverage is not full coverage. + let scan = match known_gap { + Evidence::Complete if !recorded => Evidence::Incomplete { + reason: EvidenceGap::CoverageUnrecorded, + }, + known => known, + }; + let quality = summary.quality.as_ref(); + // The dimensions the overall score weighs. An overall score always + // weighs at least one, so an empty list beside a score was not + // recorded: a missing list reads as empty here, as it does in the + // Python loader. + let assessed: Option> = quality.and_then(|quality| { + let parsed: Vec = quality + .assessed_dimensions + .iter() + // A dimension a later release added is dropped here: when it + // was sampled, its component label is unknown and + // `unknown_component_coverage` leaves the overall score + // unrecorded; when it was exact it changes no coverage. + .filter_map(|name| name.parse().ok()) + .collect(); + (!(parsed.is_empty() && quality.overall_score.is_some())).then_some(parsed) + }); + let evidence = |provenance| { + let Some(quality) = quality else { + return scan; + }; + let sampled = quality.sampled_dimensions.as_deref(); + let coverage = match (provenance, &assessed) { + // Without the weights, a sampled component may or may not + // reach the overall score; only "nothing was sampled" settles it. + (Provenance::Overall, None) if sampled.is_some_and(|s| s.is_empty()) => { + Evidence::Complete + } + (Provenance::Overall, None) => Evidence::Incomplete { + reason: EvidenceGap::CoverageUnrecorded, + }, + (_, assessed) => sampled_component_evidence( + sampled, + assessed.as_deref().unwrap_or_default(), + provenance, + ), + }; + scan.and(coverage) + }; + let dimensions = QualityDimension::all() + .into_iter() + .map(|dimension| DimensionSide { + dimension, + score: at_precision( + quality + .and_then(|quality| quality.dimension_scores.get(&dimension.to_string())) + .copied() + .flatten(), + ), + evidence: evidence(Provenance::Dimension(dimension)), + }) + .collect(); + Ok(Self { + reference: ReportReference { + id: None, + timestamp: None, + source: summary.source, + rows_processed: execution.rows_processed, + evidence: scan, + }, + quality_score: at_precision(quality.and_then(|quality| quality.overall_score)), + overall_evidence: unknown_component_coverage( + quality.and_then(|quality| quality.sampled_dimensions.as_deref()), + ) + .map_or_else(|| evidence(Provenance::Overall), |gap| scan.and(gap)), + dimensions, + columns: first_of_each_name( + summary + .columns + .into_iter() + .map(|column| (column.name, at_precision(column.null_percentage))), + ), + metric_semantics: summary.metric_semantics, + }) + } +} + +/// The fields of a flat summary a comparison reads. Everything else in the +/// document is ignored. +#[derive(serde::Deserialize)] +struct Summary { + source: String, + execution: SummaryExecution, + columns: Vec, + quality: Option, + #[serde(default)] + metric_semantics: Option, +} + +#[derive(serde::Deserialize)] +struct SummaryExecution { + rows_processed: Option, + source_exhausted: Option, + truncation_reason: Option, + sampling_applied: Option, + error_count: Option, +} + +#[derive(serde::Deserialize)] +struct SummaryColumn { + name: String, + null_percentage: Option, +} + +#[derive(serde::Deserialize)] +struct SummaryQuality { + overall_score: Option, + #[serde(default)] + dimension_scores: BTreeMap>, + #[serde(default)] + assessed_dimensions: Vec, + /// Absent in a summary written before dataprof recorded it, which reads + /// as unrecorded coverage, never as "nothing was sampled". + sampled_dimensions: Option>, +} + +#[cfg(test)] +mod tests { + use super::*; + + fn complete() -> Sides { + Sides { + a: Evidence::Complete, + b: Evidence::Complete, + } + } + + #[test] + fn a_zero_baseline_has_an_absolute_change_and_no_relative_one() { + let change = ScoreChange::new(Some(0.0), Some(50.0), complete()); + assert_eq!(change.abs, Some(50.0)); + assert_eq!(change.rel_pct, None); + } + + #[test] + fn a_change_is_rounded_to_the_serialized_precision() { + // 80.1 - 80.0 is 0.0999... in binary; the contract states 0.1. The + // relative change divides that same unrounded difference, as the + // Python implementation that shipped did: 0.1249..., so 0.12. + let change = ScoreChange::new(Some(80.0), Some(80.1), complete()); + assert_eq!(change.abs, Some(0.1)); + assert_eq!(change.rel_pct, Some(0.12)); + } + + #[test] + fn a_missing_side_leaves_both_changes_absent() { + for (a, b) in [(None, Some(1.0)), (Some(1.0), None), (None, None)] { + let change = ScoreChange::new(a, b, complete()); + assert_eq!((change.abs, change.rel_pct), (None, None)); + } + } + + #[test] + fn a_non_finite_input_is_no_number() { + assert_eq!(at_precision(Some(f64::NAN)), None); + assert_eq!(at_precision(Some(f64::INFINITY)), None); + assert_eq!(at_precision(Some(12.345)), Some(12.35)); + } +} diff --git a/crates/dataprof-runtime/src/streaming_stats.rs b/crates/dataprof-runtime/src/streaming_stats.rs index 18e83139..6963a401 100644 --- a/crates/dataprof-runtime/src/streaming_stats.rs +++ b/crates/dataprof-runtime/src/streaming_stats.rs @@ -462,7 +462,21 @@ impl RowSignature { /// ending in one of those against a row that ends in no field at all, /// inventing a duplicate. pub fn push_field(&mut self, value: &str) { - let _ = write!(self.buffer, "{}:", value.len()); + self.push_tagged(value, ':'); + } + + /// Append one field holding a JSON object or array, given as its text. + /// + /// A container crosses as its compact JSON, which a string can spell, so + /// it is tagged apart from a scalar: `"{\"x\":1}"` and `{"x": 1}` are two + /// values and must not sign alike (#945). + pub fn push_container_field(&mut self, text: &str) { + self.push_tagged(text, '#'); + } + + /// Append one field as its length, `tag`, then the text. + fn push_tagged(&mut self, value: &str, tag: char) { + let _ = write!(self.buffer, "{}{tag}", value.len()); self.buffer.push_str(value); if !value.is_empty() { self.len_through_last_present = self.buffer.len(); @@ -702,6 +716,22 @@ impl StreamingColumnCollection { pub fn process_record(&mut self, headers: &[String], values: I) where I: IntoIterator, + { + self.process_record_with_containers(headers, values, &[]); + } + + /// [`process_record`](Self::process_record) for a record whose fields at + /// `containers` (positions in `headers`) were JSON objects or arrays, + /// given as their text. Their statistics read the text like any other + /// value; only the row signature tells them apart from a string that + /// spells the same text (#945). + pub fn process_record_with_containers( + &mut self, + headers: &[String], + values: I, + containers: &[usize], + ) where + I: IntoIterator, { let mut row_signature = RowSignature::default(); let mut row_has_null = false; @@ -710,9 +740,13 @@ impl StreamingColumnCollection { // Headers define the row schema. Normalize a missing trailing field to // the profiler's empty/null representation so ragged flexible records // update every column and hash identically to an explicit empty field. - for header in headers { + for (position, header) in headers.iter().enumerate() { let value = values.next().unwrap_or_default(); - row_signature.push_field(&value); + if containers.contains(&position) { + row_signature.push_container_field(&value); + } else { + row_signature.push_field(&value); + } if let Some(stats) = self.columns.get_mut(header) { stats.update(&value); diff --git a/crates/dataprof/src/lib.rs b/crates/dataprof/src/lib.rs index 5c494373..0f41ae2f 100644 --- a/crates/dataprof/src/lib.rs +++ b/crates/dataprof/src/lib.rs @@ -65,6 +65,10 @@ pub use dataprof_runtime::{ QualityAnalysisStatus, QualityPolicy, REPORT_SCHEMA_VERSION, RequiredMetric, Severity, UnevaluatedRule, Verdict, }; +pub use dataprof_runtime::{ + ColumnChange, DimensionChange, ReportDelta, ReportReference, ReportSide, SchemaChange, + ScoreChange, SemanticsComparison, Sides, +}; #[cfg(feature = "async-streaming")] pub use dataprof_engines::streaming::ReqwestSource; diff --git a/crates/dataprof/src/profiler.rs b/crates/dataprof/src/profiler.rs index 5fda963e..4a7ec92e 100644 --- a/crates/dataprof/src/profiler.rs +++ b/crates/dataprof/src/profiler.rs @@ -67,6 +67,8 @@ pub struct ProfilerConfig { pub identifier_columns: Vec, /// Columns whose values should contribute to timeliness quality metrics. pub temporal_columns: Vec, + /// Start/end column pairs timeliness compares as given. + pub temporal_pairs: Vec<(String, String)>, /// Database connection configuration. Required for `analyze_query()`. #[cfg(feature = "database")] pub database_config: Option, @@ -92,6 +94,7 @@ impl Default for ProfilerConfig { positive_columns: Vec::new(), identifier_columns: Vec::new(), temporal_columns: Vec::new(), + temporal_pairs: Vec::new(), #[cfg(feature = "database")] database_config: None, } @@ -311,6 +314,14 @@ impl Profiler { self } + /// Declare start/end column pairs for timeliness to compare, such as + /// `valid_from`/`valid_until`. Declared pairs are compared as given; the + /// other temporal columns are still paired by name. + pub fn temporal_pairs(mut self, pairs: Vec<(String, String)>) -> Self { + self.config.temporal_pairs = pairs; + self + } + /// Set the progress update interval (default: 500ms) pub fn progress_interval(mut self, interval: Duration) -> Self { self.config.progress_interval = interval; @@ -342,6 +353,12 @@ impl Profiler { } /// Set an async channel for progress events (requires `async-streaming` feature). + /// + /// Intermediate events (such as [`ProgressEvent::ChunkProcessed`]) are best-effort + /// and dropped if the channel is full rather than blocking profiling. + /// The terminal [`ProgressEvent::Finished`] event uses a slot reserved up front via + /// `try_reserve_owned()`, guaranteeing delivery without blocking even when the consumer + /// drains the channel only after profiling completes. #[cfg(feature = "async-streaming")] pub fn progress_channel(mut self, tx: tokio::sync::mpsc::Sender) -> Self { self.progress_sink = ProgressSink::Channel(tx); @@ -385,6 +402,12 @@ impl Profiler { // finish. A sink on `Auto` routes CSV to the incremental engine. let bracket = self.progress_sink.is_active() && !(is_csv && !matches!(self.config.engine, EngineType::Columnar)); + #[cfg(feature = "async-streaming")] + let terminal_permit = if bracket { + self.progress_sink.reserve_terminal() + } else { + None + }; let source_bytes = std::fs::metadata(path).ok().map(|meta| meta.len()); let started = std::time::Instant::now(); if bracket { @@ -419,12 +442,20 @@ impl Profiler { } else { 0 }); - self.progress_sink.emit(ProgressEvent::Finished { + let event = ProgressEvent::Finished { total_rows: execution.rows_processed, total_bytes, elapsed: started.elapsed(), truncated: !execution.source_exhausted, - }); + }; + #[cfg(feature = "async-streaming")] + if let Some(permit) = terminal_permit { + permit.send(event); + } else { + self.progress_sink.emit(event); + } + #[cfg(not(feature = "async-streaming"))] + self.progress_sink.emit(event); } Ok(report) } @@ -543,6 +574,7 @@ impl Profiler { self.config.identifier_columns.clone(), ) .with_temporal_columns(self.config.temporal_columns.clone()) + .with_temporal_pairs(self.config.temporal_pairs.clone()) } /// Reject semantic hints that could not bind to the profiled data. diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 837f0c3d..24ad6480 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -188,7 +188,8 @@ Measures duplication in the data. - `duplicate_rows` -- number of exact duplicate rows. A nested value compares as a value, so two objects, or two Arrow maps, with the same entries in a - different order are equal + different order are equal, and a string never equals an object or array, + even one that spells its JSON - `key_uniqueness` -- ratio of distinct values in likely-key columns - `high_cardinality_warning` -- flagged when a column has suspiciously many unique values @@ -212,7 +213,10 @@ Measures whether confidently inferred date/time values are current and consistent. Inferred date columns participate automatically. Pass `temporal_columns=["created_at", "updated_at"]` in Python or call `.temporal_columns(...)` on the Rust profiler to add columns whose dates cannot -be inferred confidently, such as mixed-format strings. +be inferred confidently, such as mixed-format strings. Pass +`temporal_pairs=[("valid_from", "valid_until")]` in Python or call +`.temporal_pairs(...)` on the Rust profiler to compare start/end pairs that no +role word below pairs by name. - `future_dates_count` -- dates that are in the future - `stale_data_ratio` -- fraction of temporal data that appears outdated @@ -223,7 +227,8 @@ be inferred confidently, such as mixed-format strings. With several start or end columns, a start pairs with the end whose other words match (`trip_start` with `trip_end`), or failing that, whose other words match apart from a trailing date word (`trip_start_date` with - `trip_end`); when that is ambiguous, no pair is formed + `trip_end`); when that is ambiguous, no pair is formed unless it is declared + in `temporal_pairs` ### Validity diff --git a/docs/python/README.md b/docs/python/README.md index d281e6f9..60f90ea6 100644 --- a/docs/python/README.md +++ b/docs/python/README.md @@ -110,6 +110,7 @@ dp.profile( positive_columns=None, # list[str] -- columns expected to be non-negative identifier_columns=None, # list[str] -- semantic IDs, not measures temporal_columns=None, # list[str] -- columns assessed for timeliness + temporal_pairs=None, # list[tuple[str, str]] -- start/end pairs compared as given ) -> ProfileReport ``` @@ -472,7 +473,10 @@ boolean columns with no parsed booleans have no statistics block. Their statisti accessors return `None`, including boolean counts and `true_ratio`. The declared column type, row/null counts, and numeric `invalid_count` remain available. A measured numeric zero or an all-false column still reports `0.0`. CSV columns -containing only nulls infer as text because CSV carries no declared type. +containing only nulls infer as text because CSV carries no declared type. A +column with no non-null value has no statistics block whatever its type, so an +all-null text column's `min_length`, `max_length` and `avg_length` are `None`, +not `0`. This uses the existing absent-statistics representation (`ColumnStats::None` in Rust); numeric fields do not become optional within `NumericStats`, and the report @@ -652,15 +656,20 @@ marks numeric-looking IDs as semantic strings so numeric stats and outlier metrics do not treat them as measures. Timeliness scoring assesses confidently inferred date columns by default. `temporal_columns` adds columns that inference cannot identify confidently, such -as mixed-format date strings. +as mixed-format date strings. `temporal_pairs=[("valid_from", "valid_until")]` +declares start/end pairs that timeliness compares row by row, for names no role +word pairs; the remaining temporal columns are still paired by name. Hints are validated, never silently dropped. A hint that names a missing column raises `ValueError` listing the unmatched names and the available columns; a `positive_columns` hint on a column with no numeric values, or a -`temporal_columns` hint on a column with no dates, is likewise rejected. +`temporal_columns` hint on a column with no dates, is likewise rejected. A +`temporal_pairs` entry is checked the same way: a missing column, or a column +with no dates, raises `ValueError` naming `temporal_pairs`. `report.semantic_hint_bindings` records how each hint bound — `column`, `kind`, `checked_values`, `matched_values`, and `exact` (whether the counts covered -every row or a sample): +every row or a sample). Each column of a declared pair is recorded once, with +kind `temporal_pair`: ```python report = dp.profile("readings.csv", positive_columns=["pressure"]) @@ -1189,30 +1198,45 @@ checkout with `uv sync` and `uv run maturin develop`, then invoke the module wit ### `compare()` -Detect quality drift or schema changes between two profiles (e.g. the same -dataset before and after a pipeline run): +`a.compare(b)` returns what changed from `a` to `b`, such as a saved baseline +and today's run. It is computed by the Rust `ReportDelta`, so both languages +return the same document: ```python -before = dp.profile("data_v1.csv") -after = dp.profile("data_v2.csv") -delta = before.compare(after) -# { -# "quality_score": {"a": 92.3, "b": 88.1, "abs": -4.2, "rel_pct": -4.55}, -# "dimensions": {"completeness": {...}, "consistency": {...}, ...}, -# "columns": {"email": {"null_pct_a": 1.0, "null_pct_b": 6.5, "null_pct_delta": 5.5}, ...}, -# "schema": {"added": ["phone"], "removed": [], "common": ["id", "email", ...]}, -# "metric_semantics": {"a": {...}, "b": {...}, "comparable": True}, -# } +baseline = dp.ProfileReport.load("baseline.json") +delta = baseline.compare(dp.profile("orders.csv")) + +delta["quality_score"] # {"a": 91.2, "b": 88.75, "abs": -2.45, "rel_pct": -2.69, "evidence": {...}} +delta["dimensions"]["completeness"] # the same shape, for each of the seven dimensions +delta["columns"]["email"] # {"null_pct_a": 1.0, "null_pct_b": 4.5, "null_pct_delta": 3.5} +delta["schema"] # {"added": [...], "removed": [...], "common": [...]} +delta["metric_semantics"]["comparable"] # True, False, or None when either side predates the record +delta["reports"]["b"] # {"id", "timestamp", "source", "rows_processed", "evidence"} ``` -`metric_semantics.comparable` is `True` when both reports record every -measurement definition this release knows, with the same values, and `None` -when either side does not, which includes every report written before 0.12. Text lengths counted UTF-8 bytes through 0.11, -so across that boundary a changed `max_length` on non-ASCII text can be the unit -rather than the data. - -> The `compare()` result shape is provisional and will align with the Rust-side -> `QualityDelta` type once it lands. +- **Signs.** Every change is `b - a`. A positive score change is an + improvement; a positive `null_pct_delta` is more nulls. +- **Precision.** Both sides are compared at their serialized 2-decimal + precision, so a report and its saved file compare alike. +- **No invented numbers.** A score either side did not assess, or a column one + side lacks or that holds no values, leaves the change `None`. `rel_pct` is + `None` when `a` is 0. +- **Evidence.** Each score carries the coverage behind it on both sides, with + the reasons `check()` uses (`truncated`, `sampled`, `records_skipped`, + `quality_sampled`, `coverage_unrecorded`). A change between an exact + baseline and a sampled or truncated run can be a sampling difference. +- **Definitions.** Only `comparable: True` means a change in a measurement + is a change in the data. Text lengths counted UTF-8 bytes through 0.11, so + across that boundary a changed `max_length` on non-ASCII text can be the + unit rather than the data. `comparable` covers how dataprof measures, not + how each report was configured: profile both sides with the same options + (`metrics`, `positive_columns`, `temporal_columns`, ...) before reading a + change as drift. + +Either side may be a report loaded from a flat `to_dict()` summary, including +one saved before 0.12; its `id` and `timestamp` are `None`, and coverage it +did not record reads as `coverage_unrecorded`. The shape is stable: later +releases may add keys, but do not remove or rename them. ### `save()` diff --git a/docs/schema/profile-report.v1.schema.json b/docs/schema/profile-report.v1.schema.json index cea100d2..b3c7f031 100644 --- a/docs/schema/profile-report.v1.schema.json +++ b/docs/schema/profile-report.v1.schema.json @@ -2466,6 +2466,11 @@ "const": "temporal", "description": "A [`SemanticHints::temporal_columns`] entry.", "type": "string" + }, + { + "const": "temporal_pair", + "description": "A column named by a [`SemanticHints::temporal_pairs`] entry.", + "type": "string" } ] }, diff --git a/examples/before_after_cleaning.rs b/examples/before_after_cleaning.rs index 17d12b5a..9a575cc0 100644 --- a/examples/before_after_cleaning.rs +++ b/examples/before_after_cleaning.rs @@ -17,7 +17,7 @@ use std::io::Write; use std::path::Path; use anyhow::Result; -use dataprof::{ColumnProfile, ProfileReport, Profiler}; +use dataprof::{ColumnProfile, ProfileReport, Profiler, ReportDelta}; const RAW: &str = "\ signup_id,email,plan,monthly_eur @@ -48,10 +48,6 @@ fn profile(path: &Path) -> Result { .analyze_file(path)?) } -fn null_percentage(col: &ColumnProfile) -> f64 { - 100.0 * col.null_count as f64 / col.total_count.max(1) as f64 -} - fn by_name(report: &ProfileReport) -> BTreeMap<&str, &ColumnProfile> { report .column_profiles @@ -78,36 +74,50 @@ fn main() -> Result<()> { let before: ProfileReport = serde_json::from_reader(std::fs::File::open(&baseline_path)?)?; let after = profile(&clean_path)?; + // The delta never invents a number: a score either side did not assess + // stays `None`, so print it as such instead of as zero. + let delta = ReportDelta::between(&before, &after); + let show = |value: Option| value.map_or("n/a".to_string(), |v| format!("{v:.1}")); + let quality = &delta.quality_score; + println!("## Cleaning summary\n"); println!( - "| | rows | quality |\n|---|---:|---:|\n| before | {} | {:.1} |\n| after | {} | {:.1} |", + "| | rows | quality |\n|---|---:|---:|\n| before | {} | {} |\n| after | {} | {} |", before.execution.rows_processed, - before.quality_score().unwrap_or(0.0), + show(quality.a), after.execution.rows_processed, - after.quality_score().unwrap_or(0.0), + show(quality.b), ); let dropped = before.execution.rows_processed - after.execution.rows_processed; - let gained = after.quality_score().unwrap_or(0.0) - before.quality_score().unwrap_or(0.0); - println!("\n{dropped} row(s) dropped, {gained:+.1} quality points\n"); + let gained = quality + .abs + .map_or("n/a".to_string(), |change| format!("{change:+.1}")); + println!("\n{dropped} row(s) dropped, {gained} quality points\n"); println!("## Per-column null rate\n"); println!("| column | before | after | delta |"); println!("|---|---:|---:|---:|"); - let (before_cols, after_cols) = (by_name(&before), by_name(&after)); - for (name, before_col) in &before_cols { - let Some(after_col) = after_cols.get(name) else { - println!( - "| {name} | {:.0}% | *dropped* | |", - null_percentage(before_col) - ); + let pct = |value: Option| value.map_or("n/a".to_string(), |v| format!("{v:.0}%")); + for column in &delta.columns { + let name = &column.name; + if delta.schema.removed.contains(name) { + println!("| {name} | {} | *dropped* | |", pct(column.null_pct_a)); continue; - }; - let (b, a) = (null_percentage(before_col), null_percentage(after_col)); - println!("| {name} | {b:.0}% | {a:.0}% | {:+.0}pp |", a - b); + } + let change = column + .null_pct_delta + .map_or(String::new(), |change| format!("{change:+.0}pp")); + println!( + "| {name} | {} | {} | {change} |", + pct(column.null_pct_a), + pct(column.null_pct_b) + ); } + let after_cols = by_name(&after); + // The headline numbers can improve while a specific defect survives. Check the // defects you set out to fix, not just the score. println!("\n## Did the defects go away?\n"); diff --git a/python/dataprof/_api.py b/python/dataprof/_api.py index 8deabb91..1cc50250 100644 --- a/python/dataprof/_api.py +++ b/python/dataprof/_api.py @@ -104,6 +104,7 @@ def profile_file( positive_columns: list[str] | None = None, identifier_columns: list[str] | None = None, temporal_columns: list[str] | None = None, + temporal_pairs: list[tuple[str, str]] | None = None, ) -> ProfileReport: """Profile a file path and return a report. @@ -181,6 +182,10 @@ def profile_file( identifiers instead of measures. temporal_columns: Columns whose values should contribute to timeliness quality metrics. + temporal_pairs: ``(start, end)`` column pairs whose dates timeliness + compares row by row, such as ``("valid_from", "valid_until")``. + Declared pairs are compared as given; the remaining temporal + columns are still paired by name. Returns: ProfileReport with analysis results and quality metrics. @@ -208,6 +213,7 @@ def profile_file( positive_columns=positive_columns, identifier_columns=identifier_columns, temporal_columns=temporal_columns, + temporal_pairs=temporal_pairs, ) rust_report = _analyze_file(normalized_path, config) return ProfileReport(rust_report) @@ -236,6 +242,7 @@ def profile( positive_columns: list[str] | None = None, identifier_columns: list[str] | None = None, temporal_columns: list[str] | None = None, + temporal_pairs: list[tuple[str, str]] | None = None, ) -> ProfileReport: """Profile a data source and return a report. @@ -323,6 +330,10 @@ def profile( identifiers instead of measures. temporal_columns: Columns whose values should contribute to timeliness quality metrics. + temporal_pairs: ``(start, end)`` column pairs whose dates timeliness + compares row by row, such as ``("valid_from", "valid_until")``. + Declared pairs are compared as given; the remaining temporal + columns are still paired by name. Returns: ProfileReport with analysis results and quality metrics. @@ -353,6 +364,7 @@ def profile( positive_columns=positive_columns, identifier_columns=identifier_columns, temporal_columns=temporal_columns, + temporal_pairs=temporal_pairs, ) # DataFrame/Arrow paths — build config for metric packs + quality dims + locale @@ -369,6 +381,7 @@ def _df_config() -> ProfilerConfig | None: positive_columns, identifier_columns, temporal_columns, + temporal_pairs, ) ): return ProfilerConfig( @@ -380,6 +393,7 @@ def _df_config() -> ProfilerConfig | None: positive_columns=positive_columns, identifier_columns=identifier_columns, temporal_columns=temporal_columns, + temporal_pairs=temporal_pairs, ) return None diff --git a/python/dataprof/_api.pyi b/python/dataprof/_api.pyi index 2e9e254c..ce1cea21 100644 --- a/python/dataprof/_api.pyi +++ b/python/dataprof/_api.pyi @@ -39,6 +39,7 @@ def profile( positive_columns: list[str] | None = None, identifier_columns: list[str] | None = None, temporal_columns: list[str] | None = None, + temporal_pairs: list[tuple[str, str]] | None = None, ) -> ProfileReport: """Profile a data source (file path, DataFrame, or Arrow object).""" ... @@ -65,6 +66,7 @@ def profile_file( positive_columns: list[str] | None = None, identifier_columns: list[str] | None = None, temporal_columns: list[str] | None = None, + temporal_pairs: list[tuple[str, str]] | None = None, ) -> ProfileReport: """Profile a file path with file-oriented options.""" ... diff --git a/python/dataprof/_columns.pyi b/python/dataprof/_columns.pyi index cd49ddc3..0b8a9a57 100644 --- a/python/dataprof/_columns.pyi +++ b/python/dataprof/_columns.pyi @@ -11,6 +11,8 @@ def column_to_dict(col: ColumnProfile | _NativeColumn) -> dict[str, Any]: """Convert a ColumnProfile to the nested dict layout used in ``report.to_dict()['columns']``.""" ... +_LEXICAL_CLASS_ORDER: tuple[str, ...] + def _homogeneity_counts(value: _Any) -> dict[str, int] | None: ... def _type_mixture(col: ColumnProfile) -> list[tuple[str, int, float]]: ... def _column_record(col: ColumnProfile) -> dict[str, _Any]: ... diff --git a/python/dataprof/_dataprof.pyi b/python/dataprof/_dataprof.pyi index 09c69778..71682a4b 100644 --- a/python/dataprof/_dataprof.pyi +++ b/python/dataprof/_dataprof.pyi @@ -39,6 +39,7 @@ class ProfilerConfig: identifier_columns: list[str] | None = None, temporal_columns: list[str] | None = None, columns: list[str] | None = None, + temporal_pairs: list[tuple[str, str]] | None = None, ) -> None: ... @property def engine(self) -> str: ... @@ -61,6 +62,8 @@ class ProfilerConfig: @property def temporal_columns(self) -> list[str]: ... @property + def temporal_pairs(self) -> list[tuple[str, str]]: ... + @property def jsonl_on_error(self) -> str: ... # --- Sampling --- @@ -411,6 +414,7 @@ class ProfileReport: def analyze_file(path: str, config: ProfilerConfig | None) -> ProfileReport: ... def list_patterns(locale: str | None = None) -> list[dict[str, Any]]: ... +def report_delta_json(a: ProfileReport | str, b: ProfileReport | str) -> str: ... def profile_dataframe( df: Any, name: str, max_rows: int | None, config: ProfilerConfig | None ) -> ProfileReport: ... diff --git a/python/dataprof/_inputs.py b/python/dataprof/_inputs.py index 91ebc3b5..31868119 100644 --- a/python/dataprof/_inputs.py +++ b/python/dataprof/_inputs.py @@ -87,6 +87,69 @@ def _cell_to_str(value: object) -> str | None: return str(value) +def _pandas_to_pyarrow_table(df: _Any, pyarrow: _Any) -> _Any: + """Convert a pandas DataFrame to a pyarrow Table, handling mixed object columns. + + Fast path: delegates directly to ``pyarrow.Table.from_pandas(df)``. + Fallback path: for object and categorical columns whose values Arrow cannot + cast into a single type (e.g. numeric with placeholder text or mixed + booleans/dates), converts their cells via ``_cell_to_str`` into string arrays + so they can be profiled with ``type_homogeneity`` rather than aborting (#976). + """ + try: + return pyarrow.Table.from_pandas(df) + except (pyarrow.ArrowInvalid, pyarrow.ArrowTypeError): + # If columns have duplicate labels, pyarrow Table.from_pandas fails with + # ValueError; duplicate column frames cannot be converted. + if hasattr(df.columns, "is_unique") and not df.columns.is_unique: + raise + + failing_cols: set[_Any] = set() + for i in range(len(df.columns)): + col = df.iloc[:, i] + # CategoricalDtype is an extension dtype whose name is "category". + # It can also hold mixed Python values that Arrow cannot cast. + if col.dtype != object and getattr(col.dtype, "name", None) != "category": + continue + name = df.columns[i] + try: + pyarrow.Array.from_pandas(col) + except (pyarrow.ArrowInvalid, pyarrow.ArrowTypeError): + failing_cols.add(name) + + if not failing_cols: + raise + + df_fixed = df.copy(deep=False) + for name in failing_cols: + col = df[name] + try: + if hasattr(col, "isna"): + missing = col.isna() + df_fixed[name] = [ + None if is_m else _cell_to_str(v) for is_m, v in zip(missing, col) + ] + else: + df_fixed[name] = [_cell_to_str(v) for v in col] + except Exception as exc: + raise ValueError( + f"pandas DataFrame: column {name!r} could not be converted for profiling: {exc}" + ) from exc + + try: + return pyarrow.Table.from_pandas(df_fixed) + except Exception as exc: + for name in failing_cols: + try: + pyarrow.Array.from_pandas(df_fixed[name]) + except (pyarrow.ArrowInvalid, pyarrow.ArrowTypeError) as col_exc: + raise ValueError( + f"pandas DataFrame: column {name!r} could not be converted for profiling: " + f"{col_exc}" + ) from col_exc + raise ValueError(f"pandas DataFrame conversion failed: {exc}") from exc + + def _columns_from_dict(source: dict[_Any, _Any]) -> list[_Column]: """Convert a dict of equal-length sequences into columns.""" columns: list[_Column] = [] diff --git a/python/dataprof/_profiler.py b/python/dataprof/_profiler.py index 154c975e..c41a7bc0 100644 --- a/python/dataprof/_profiler.py +++ b/python/dataprof/_profiler.py @@ -161,6 +161,11 @@ def temporal_columns(self, columns: list[str]) -> Profiler: self._kwargs["temporal_columns"] = columns return self + def temporal_pairs(self, pairs: list[tuple[str, str]]) -> Profiler: + """Declare start/end column pairs for timeliness to compare as given.""" + self._kwargs["temporal_pairs"] = pairs + return self + def metrics(self, packs: list[str]) -> Profiler: """Select metric packs to compute. diff --git a/python/dataprof/_profiler.pyi b/python/dataprof/_profiler.pyi index 28c6598e..03c8cf5d 100644 --- a/python/dataprof/_profiler.pyi +++ b/python/dataprof/_profiler.pyi @@ -39,4 +39,5 @@ class Profiler: def positive_columns(self, columns: list[str]) -> Profiler: ... def identifier_columns(self, columns: list[str]) -> Profiler: ... def temporal_columns(self, columns: list[str]) -> Profiler: ... + def temporal_pairs(self, pairs: list[tuple[str, str]]) -> Profiler: ... def profile(self, source: Any) -> ProfileReport: ... diff --git a/python/dataprof/_report.py b/python/dataprof/_report.py index 0ef75213..9820f6e7 100644 --- a/python/dataprof/_report.py +++ b/python/dataprof/_report.py @@ -8,8 +8,10 @@ import html as _html import json as _json import os as _os +import tempfile as _tempfile import warnings as _warnings from collections.abc import ( + Callable as _Callable, Iterator as _Iterator, Mapping as _Mapping, Sequence as _Sequence, @@ -18,7 +20,7 @@ from ._accessors import ColumnProfile, DataQualityMetrics, _NativeAccessor, _ReportView from ._columns import _column_record, _dominant_pattern, column_to_dict -from ._dataprof import ProfileReport as _RustProfileReport +from ._dataprof import ProfileReport as _RustProfileReport, report_delta_json as _report_delta_json from ._findings import FindingsResult as _FindingsResult, _FindingPolicy from ._gate import QualityGateResult as _QualityGateResult, _Policy from ._paths import _normalize_pathlike @@ -34,7 +36,7 @@ _section_min_cost, _stats_cell, ) -from ._report_backing import _DEFAULT_SCORE_WEIGHTS, _METRIC_SEMANTICS, _report_from_dict +from ._report_backing import _DEFAULT_SCORE_WEIGHTS, _report_from_dict from ._report_schema import _QUALITY_DIMENSIONS, REPORT_SCHEMA_VERSION from ._rounding import _r2, _r4, _round_dimension, _round_quartiles @@ -51,6 +53,57 @@ def _quality_status_document(state: str, error: str | None) -> dict[str, _Any]: return document +def _atomic_write(path: str, writer: _Callable[[str], None]) -> None: + path = _os.path.realpath(path) + directory = _os.path.dirname(_os.path.abspath(path)) + basename = _os.path.basename(path) + + fd, temp_path = _tempfile.mkstemp( + dir=directory, + prefix=f".{basename}.", + suffix=".tmp", + ) + _os.close(fd) + + try: + writer(temp_path) + + try: + mode = _os.stat(path).st_mode & 0o777 + except FileNotFoundError: + mode_path = f"{temp_path}.mode" + mode_fd = _os.open( + mode_path, + _os.O_WRONLY | _os.O_CREAT | _os.O_EXCL, + 0o666, + ) + try: + mode = _os.fstat(mode_fd).st_mode & 0o777 + finally: + _os.close(mode_fd) + _os.unlink(mode_path) + + _os.chmod(temp_path, mode) + _os.replace(temp_path, path) + finally: + try: + _os.unlink(temp_path) + except FileNotFoundError: + pass + + +def _atomic_write_text( + path: str, + content: str, + newline: str | None = None, +) -> None: + def write(temp_path: str) -> None: + with open(temp_path, "w", encoding="utf-8", newline=newline) as f: + f.write(content) + + _atomic_write(path, write) + + class ProfileReport: """Report access and export methods shared by native and restored backings. @@ -259,11 +312,11 @@ def semantic_hint_bindings(self) -> list[dict[str, _Any]]: """Per-column evidence of how each semantic hint bound to the data. Empty unless ``positive_columns``/``identifier_columns``/ - ``temporal_columns`` were supplied. Each entry has ``column``, ``kind``, - ``checked_values``, ``matched_values``, and ``exact``. A hint proven - inert over the full data raises before a report is returned, so a report - only ever carries bindings that matched something or whose evidence was - sampled. + ``temporal_columns``/``temporal_pairs`` were supplied. Each entry has + ``column``, ``kind``, ``checked_values``, ``matched_values``, and + ``exact``. A hint proven inert over the full data raises before a report + is returned, so a report only ever carries bindings that matched + something or whose evidence was sampled. """ return list(self._report.semantic_hint_bindings) @@ -739,30 +792,38 @@ def save(self, path: str | _os.PathLike[str]) -> ProfileReport: """ path = _normalize_pathlike(path) lower_path = path.lower() + if lower_path.endswith(".json"): - with open(path, "w", encoding="utf-8", newline="\n") as f: - f.write(self.to_json()) + _atomic_write_text(path, self.to_json(), newline="\n") + elif lower_path.endswith(".csv"): records = self._records() + # Opening unconditionally matters for a zero-column report: save() # must either create the requested artifact or fail, never return # success while leaving no file behind. - with open(path, "w", encoding="utf-8", newline="") as f: - if records: - writer = _csv.DictWriter(f, fieldnames=records[0].keys()) - writer.writeheader() - writer.writerows(records) + def write_csv(temp_path: str) -> None: + with open(temp_path, "w", encoding="utf-8", newline="") as f: + if records: + writer = _csv.DictWriter(f, fieldnames=records[0].keys()) + writer.writeheader() + writer.writerows(records) + + _atomic_write(path, write_csv) elif lower_path.endswith(".parquet"): table = self.to_arrow() import pyarrow.parquet as pq - pq.write_table(table, path) + def write_parquet(temp_path: str) -> None: + pq.write_table(table, temp_path) + + _atomic_write(path, write_parquet) + elif lower_path.endswith(".html"): - with open(path, "w", encoding="utf-8") as f: - f.write(self.to_html()) + _atomic_write_text(path, self.to_html()) + elif lower_path.endswith((".md", ".markdown")): - with open(path, "w", encoding="utf-8") as f: - f.write(self.to_markdown()) + _atomic_write_text(path, self.to_markdown()) else: raise ValueError( f"Unsupported format for '{path}'. " @@ -955,18 +1016,21 @@ def _to_llm_context(self, *, source: str, max_tokens: int, include_samples: bool return "\n".join([*header, *body]) def compare(self, other: ProfileReport) -> dict[str, _Any]: - """Compare this report with another and return a dict of deltas. + """Compare this report (``a``) with ``other`` (``b``) and return the deltas. The result captures quality drift and schema differences between two - profiles (e.g. the same dataset before and after a pipeline change): - - - ``quality_score``: overall score for each side plus absolute and - relative-percent change. - - ``dimensions``: the same a/b/abs/rel_pct shape per ISO 25012 - dimension (completeness, consistency, uniqueness, accuracy, - timeliness, validity, precision), sourced from :meth:`quality_summary`. - - ``columns``: per-column null-percentage drift over the union of - column names (missing on one side → ``None``). + profiles, such as a saved baseline and today's run. It is computed by + the Rust ``ReportDelta``, so Rust and Python return the same document: + + - ``quality_score``: ``a``, ``b``, ``abs`` (``b - a`` in points) and + ``rel_pct`` (``b - a`` as a percentage of ``a``), plus ``evidence``, + the coverage behind each side's score. + - ``dimensions``: the same shape for each ISO 25012 dimension + (completeness, consistency, uniqueness, accuracy, timeliness, + validity, precision), as :meth:`quality_summary` reports them. + - ``columns``: per-column ``null_pct_a``, ``null_pct_b`` and + ``null_pct_delta`` over the union of column names, this report's + order first. - ``schema``: column names ``added`` / ``removed`` / ``common``. - ``metric_semantics``: each side's :attr:`metric_semantics` and ``comparable``: ``True`` when both record every definition this @@ -974,92 +1038,33 @@ def compare(self, other: ProfileReport) -> dict[str, _Any]: does not, which includes every report written before 0.12. Only ``True`` means a difference in a measurement, such as a text length, is a difference in the data. - - .. note:: - The exact shape is provisional and will align with the Rust-side - ``QualityDelta`` type (#310) once it lands. + - ``reports``: for each side, its ``id`` and ``timestamp`` (``None`` + for a flat summary), ``source``, ``rows_processed`` and the scan's + ``evidence``. + + Every change is ``b - a``: a positive score change is an improvement, + a positive ``null_pct_delta`` is more nulls. Numbers are compared at + their serialized 2-decimal precision. A number either side lacks + leaves the change ``None``, and so does ``rel_pct`` when ``a`` is 0. + + Each ``evidence`` is ``{"coverage": "complete"}``, or + ``{"coverage": "incomplete", "reason": ...}`` with the reason + ``check()`` uses: ``truncated``, ``sampled``, ``records_skipped``, + ``quality_sampled`` or ``coverage_unrecorded``. A change between an + exact and a sampled side can be a sampling difference. + + The shape is stable: later releases may add keys, but do not remove + or rename them. Either report may be loaded from a flat ``to_dict()`` + summary, including one saved before 0.12. """ + return _json.loads(_report_delta_json(self._delta_input(), other._delta_input())) - def _delta(a: float | None, b: float | None) -> dict[str, float | None]: - abs_change = None if a is None or b is None else _r2(b - a) - if a is None or b is None or a == 0: - rel_pct = None - else: - rel_pct = _r2((b - a) / abs(a) * 100.0) - return {"a": a, "b": b, "abs": abs_change, "rel_pct": rel_pct} - - a_summary = self.quality_summary() - b_summary = other.quality_summary() - - dimensions = { - dim: _delta(a_summary.get(dim), b_summary.get(dim)) - for dim in ( - "completeness", - "consistency", - "uniqueness", - "accuracy", - "timeliness", - "validity", - "precision", - ) - } - - a_cols = self.column_profiles - b_cols = other.column_profiles - a_order = list(a_cols) - b_order = list(b_cols) - a_names = set(a_order) - b_names = set(b_order) - # Preserve the left report's schema order, then append right-only - # columns in their source order. - all_names = [*a_order, *(name for name in b_order if name not in a_names)] - - def _null_pct(cols: dict[str, ColumnProfile], name: str) -> float | None: - col = cols.get(name) - return _r2(col.null_percentage) if col is not None else None - - columns: dict[str, dict[str, float | None]] = {} - for name in all_names: - null_a = _null_pct(a_cols, name) - null_b = _null_pct(b_cols, name) - null_delta = None if null_a is None or null_b is None else _r2(null_b - null_a) - columns[name] = { - "null_pct_a": null_a, - "null_pct_b": null_b, - "null_pct_delta": null_delta, - } - - a_semantics = self.metric_semantics - b_semantics = other.metric_semantics - # Both sides must record every definition this build knows; an empty - # or partial record is as unknown as a missing one. Loaded values are - # validated to the one value each definition has today, so a `False` - # needs a second recorded value to become reachable. - known = _METRIC_SEMANTICS.keys() - comparable = ( - a_semantics == b_semantics - if a_semantics is not None - and b_semantics is not None - and a_semantics.keys() == known - and b_semantics.keys() == known - else None - ) - - return { - "quality_score": _delta(self.quality_score, other.quality_score), - "dimensions": dimensions, - "columns": columns, - "metric_semantics": { - "a": a_semantics, - "b": b_semantics, - "comparable": comparable, - }, - "schema": { - "added": [name for name in b_order if name not in a_names], - "removed": [name for name in a_order if name not in b_names], - "common": [name for name in a_order if name in b_names], - }, - } + def _delta_input(self) -> _RustProfileReport | str: + # A flat summary never becomes a native report (it lacks the + # provenance one records), so it crosses as the summary it exports. + if self._native_report is not None: + return self._native_report + return _json.dumps(self._legacy_summary()) @classmethod def from_dict(cls, data: dict[str, _Any]) -> ProfileReport: diff --git a/python/dataprof/_report_backing.py b/python/dataprof/_report_backing.py index 14a5a0fe..0e44abe1 100644 --- a/python/dataprof/_report_backing.py +++ b/python/dataprof/_report_backing.py @@ -10,7 +10,7 @@ from typing import Any from ._accessors import ColumnProfile, _Field, _MappingAccessor, _ReportView -from ._columns import _homogeneity_counts +from ._columns import _LEXICAL_CLASS_ORDER, _homogeneity_counts from ._report_schema import _QUALITY_DIMENSIONS # The states a `quality_status` document may declare, and whether each one @@ -121,17 +121,30 @@ def _is_count(value: Any) -> bool: def _is_finite_number(value: Any) -> bool: # bool before int: bool is an int subclass, and True == 1. - return ( - isinstance(value, int | float) - and not isinstance(value, bool) - and math.isfinite(float(value)) - ) + if not isinstance(value, int | float) or isinstance(value, bool): + return False + try: + return math.isfinite(float(value)) + except (OverflowError, ValueError): + return False def _is_list_of_mappings(value: Any) -> bool: return isinstance(value, list) and all(isinstance(entry, dict) for entry in value) +def _is_list_of_dimension_names(value: Any) -> bool: + return isinstance(value, list) and all( + isinstance(entry, str) and entry in _QUALITY_DIMENSIONS for entry in value + ) + + +def _is_homogeneity_counts(value: Any) -> bool: + if not isinstance(value, dict): + return False + return all(name in value and _is_count(value[name]) for name in _LEXICAL_CLASS_ORDER) + + def _column_accessor(document: dict[str, Any], path: str) -> _MappingAccessor: values = dict(document) # `null` is the schema's "not analyzed", the same answer as a missing key. @@ -148,7 +161,17 @@ def _column_accessor(document: dict[str, Any], path: str) -> _MappingAccessor: for name, field in vars(ColumnProfile).items(): if isinstance(field, _Field) and field._document_section == "stats": values[name] = stats.get(name) if stats is not None else None - values["type_homogeneity"] = _homogeneity_counts(document.get("type_homogeneity")) + homogeneity = _read_field( + document, + "type_homogeneity", + f"{path}.type_homogeneity", + "a four-count mapping or null", + lambda value: value is None or _is_homogeneity_counts(value), + None, + ) + values["type_homogeneity"] = ( + _homogeneity_counts(homogeneity) if homogeneity is not None else None + ) patterns = _read_field( document, "patterns", @@ -182,7 +205,16 @@ def _column_accessor(document: dict[str, Any], path: str) -> _MappingAccessor: def _quality_accessor(document: dict[str, Any]) -> _MappingAccessor: values = dict(document) - values["overall_quality_score"] = document.get("overall_score") + overall_score = _read_field( + document, + "overall_score", + "quality.overall_score", + "a finite number or null", + lambda value: value is None or _is_finite_number(value), + None, + ) + values["overall_score"] = overall_score + values["overall_quality_score"] = float(overall_score) if overall_score is not None else None values["low_sample_warning"] = _read_field( document, "low_sample_warning", @@ -191,15 +223,17 @@ def _quality_accessor(document: dict[str, Any]) -> _MappingAccessor: _is_bool, False, ) - values["assessed_dimensions"] = _read_field( - document, - "assessed_dimensions", - "quality.assessed_dimensions", - "a list", - lambda value: isinstance(value, list), - [], + values["assessed_dimensions"] = list( + _read_field( + document, + "assessed_dimensions", + "quality.assessed_dimensions", + "a list of known dimension names", + _is_list_of_dimension_names, + [], + ) ) - values["dimension_scores"] = _read_field( + dimension_scores = _read_field( document, "dimension_scores", "quality.dimension_scores", @@ -207,6 +241,15 @@ def _quality_accessor(document: dict[str, Any]) -> _MappingAccessor: lambda value: isinstance(value, dict), dict.fromkeys(_QUALITY_DIMENSIONS), ) + read_scores = dict(dimension_scores) + for dim, score in dimension_scores.items(): + if score is not None and not _is_finite_number(score): + raise ValueError( + f"from_dict(): 'quality.dimension_scores.{dim}' must be a finite number or null, " + f"got {score!r}." + ) + read_scores[dim] = float(score) if score is not None else None + values["dimension_scores"] = read_scores values["score_weights"] = _read_score_weights(document) return _MappingAccessor(values) @@ -326,7 +369,9 @@ def _report_from_dict(document: dict[str, Any]) -> _ReportView: None, ) values["quality"] = _quality_accessor(quality) if quality is not None else None - values["quality_score"] = quality.get("overall_score") if quality is not None else None + values["quality_score"] = ( + values["quality"].get("overall_quality_score") if values["quality"] is not None else None + ) values["quality_sampled_dimensions"] = ( quality.get("sampled_dimensions") if quality is not None else None ) diff --git a/python/tests/test_all_null_columns.py b/python/tests/test_all_null_columns.py new file mode 100644 index 00000000..864342ca --- /dev/null +++ b/python/tests/test_all_null_columns.py @@ -0,0 +1,64 @@ +"""A column with no non-null value carries no statistics (#983). + +An all-null text column used to report lengths of 0, the same as a column of +real zero-length values, while an all-null numeric column already reported no +statistics. `tests/all_null_columns.rs` is the Rust twin over the file engines; +this covers the in-memory routes. The oracle (`test_oracle.py`, `all_null`) +checks the same case on every route against an independent answer. +""" + +from __future__ import annotations + +import dataprof as dp +import pytest + +pd = pytest.importorskip("pandas") +pa = pytest.importorskip("pyarrow") +pl = pytest.importorskip("polars") + +EMPTY = [None, None, None] +BLANK = ["NA", "null", ""] +NOTE = [None, "x", None] + + +def _routes(): + frame = {"id": [1, 2, 3], "empty": EMPTY, "blank": BLANK, "note": NOTE} + records = [dict(zip(frame, row)) for row in zip(*frame.values())] + table = pa.table( + { + "id": [1, 2, 3], + "empty": pa.array(EMPTY, pa.string()), + "blank": BLANK, + "note": NOTE, + } + ) + return { + "dict": lambda: dp.profile(frame), + "records": lambda: dp.profile(records), + "pandas": lambda: dp.profile(pd.DataFrame(frame)), + "arrow": lambda: dp.profile(table), + "polars": lambda: dp.profile(pl.from_arrow(table)), + } + + +def test_a_column_of_nulls_reports_no_statistics_and_keeps_its_counts(): + wrong = [] + for route, run in _routes().items(): + report = run() + for name in ("empty", "blank"): + column = report[name] + exported = report.to_dict()["columns"][list(report.column_profiles).index(name)] + if exported.get("stats") is not None or column.min_length is not None: + wrong.append(f"{route}: {name} stats {exported.get('stats')}") + if (column.null_count, column.total_count) != (3, 3): + wrong.append(f"{route}: {name} counts {column.null_count}/{column.total_count}") + note = report["note"] + if (note.min_length, note.max_length) != (1, 1): + wrong.append(f"{route}: note lengths {note.min_length}/{note.max_length}") + assert not wrong, "\n".join(wrong) + + +def test_an_identifier_column_of_nulls_reports_no_statistics(): + report = dp.profile({"id": [1, 2], "code": [None, None]}, identifier_columns=["code"]) + assert report.to_dict()["columns"][1].get("stats") is None + assert report["code"].min_length is None diff --git a/python/tests/test_canonical_report_document.py b/python/tests/test_canonical_report_document.py index b6065020..9876714e 100644 --- a/python/tests/test_canonical_report_document.py +++ b/python/tests/test_canonical_report_document.py @@ -168,6 +168,10 @@ def test_restored_quality_prints_the_score_it_returns(): def test_legacy_summary_export_agrees_with_what_was_loaded(): summary = dataprof.profile({"x": ["a", "1", None]}).to_dict() summary["columns"][0]["type_homogeneity"] = {"numeric": 1, "text": 1} + with pytest.raises(ValueError, match=r"'columns\[0\]\.type_homogeneity' must be"): + dataprof.ProfileReport.from_dict(summary) + + summary["columns"][0]["type_homogeneity"] = None restored = dataprof.ProfileReport.from_dict(summary) assert restored["x"].type_homogeneity is None for exported in (restored.to_dict(), json.loads(restored.to_json())): diff --git a/python/tests/test_flat_loader_malformed_fields.py b/python/tests/test_flat_loader_malformed_fields.py index 9293c940..0777bcc3 100644 --- a/python/tests/test_flat_loader_malformed_fields.py +++ b/python/tests/test_flat_loader_malformed_fields.py @@ -64,6 +64,18 @@ def _no_quality_status(document: dict[str, Any], value: Any) -> None: #: (field path in the error, how to place the value, malformed values) MALFORMED = [ ("columns[0].stats", _set("columns", 0, "stats"), [[1], "x", 1]), + ( + "columns[0].type_homogeneity", + _set("columns", 0, "type_homogeneity"), + [ + [1], + "x", + 1, + {"numeric": 1}, + {"boolean": "x", "date": 0, "numeric": 0, "text": 0}, + {"boolean": -1, "date": 0, "numeric": 0, "text": 0}, + ], + ), ("columns[1].patterns", _pattern_entry, [1, "x", None]), ("columns[1].patterns", _set("columns", 1, "patterns"), ["x", {}]), ( @@ -72,8 +84,22 @@ def _no_quality_status(document: dict[str, Any], value: Any) -> None: ["x", None, True, float("nan")], ), ("quality.low_sample_warning", _set("quality", "low_sample_warning"), ["false", 0, None]), - ("quality.assessed_dimensions", _set("quality", "assessed_dimensions"), [None, {}, "x"]), + ( + "quality.overall_score", + _set("quality", "overall_score"), + ["x", [1], {}, True, float("nan"), 10**309], + ), + ( + "quality.assessed_dimensions", + _set("quality", "assessed_dimensions"), + [None, {}, "x", [1], ["unknown"]], + ), ("quality.dimension_scores", _set("quality", "dimension_scores"), [[1], None, "x"]), + ( + "quality.dimension_scores.completeness", + _set("quality", "dimension_scores", "completeness"), + ["x", [1], {}, True, float("nan"), 10**309], + ), ("quality.score_bounds", _set("quality", "score_bounds"), ["x", [1]]), ("quality", _no_quality_status, ["x", [1]]), ("execution.ragged_row_count", _set("execution", "ragged_row_count"), [None, -1, "0", True]), @@ -101,7 +127,9 @@ def test_a_malformed_field_is_refused_with_its_path(summary, path, place, value) ((), "semantic_hint_bindings", lambda r: r.semantic_hint_bindings, []), (("quality",), "low_sample_warning", lambda r: r.quality.low_sample_warning, False), (("quality",), "assessed_dimensions", lambda r: r.quality.assessed_dimensions(), []), + (("quality",), "overall_score", lambda r: r.quality.overall_quality_score(), None), (("columns", 0), "stats", lambda r: r["id"].min, None), + (("columns", 0), "type_homogeneity", lambda r: r["id"].type_homogeneity, None), (("columns", 1), "patterns", lambda r: r["city"].patterns, None), ] @@ -121,12 +149,30 @@ def test_a_missing_key_keeps_its_compatibility_default(summary, section, key, re def test_null_still_means_not_analyzed_where_the_schema_allows_it(summary): document = copy.deepcopy(summary) document["columns"][0]["stats"] = None + document["columns"][0]["type_homogeneity"] = None document["columns"][1]["patterns"] = None document["quality"]["score_bounds"] = None + document["quality"]["overall_score"] = None + document["quality"]["dimension_scores"]["completeness"] = None report = dp.ProfileReport.from_dict(document) assert report["id"].min is None + assert report["id"].type_homogeneity is None assert report["city"].patterns is None + assert report.quality is not None + assert report.quality.overall_quality_score() is None + assert report.quality.dimension_scores()["completeness"] is None def test_a_well_formed_summary_round_trips(summary): assert dp.ProfileReport.from_dict(copy.deepcopy(summary)).to_dict() == summary + + +def test_unknown_additive_keys_in_type_homogeneity_are_ignored(summary): + document = copy.deepcopy(summary) + document["columns"][0]["type_homogeneity"]["classifier_provenance"] = "v2" + report = dp.ProfileReport.from_dict(document) + assert report["id"].type_homogeneity is not None + assert ( + report["id"].type_homogeneity["numeric"] + == summary["columns"][0]["type_homogeneity"]["numeric"] + ) diff --git a/python/tests/test_nested_duplicate_rows.py b/python/tests/test_nested_duplicate_rows.py index 4c9c5f94..3ba10b72 100644 --- a/python/tests/test_nested_duplicate_rows.py +++ b/python/tests/test_nested_duplicate_rows.py @@ -3,7 +3,9 @@ The JSON readers rendered an object in its source key order, so equal records with their keys in a different order were not duplicates. Arrow and Parquet rendered a nested value as its display string, which matches ``["p, q"]`` with -``["p", "q"]`` and ``[None]`` with ``[]``. +``["p", "q"]`` and ``[None]`` with ``[]``. A container's text is its compact +JSON, which a string can spell, so a string equal to it signed the same row as +the container (#945). ``tests/nested_duplicate_rows.rs`` is the Rust twin. """ @@ -66,6 +68,25 @@ 0, False, ), + # #945: a container crosses as its compact JSON, which a string can spell. + ( + "a string spelling an object against the object", + [{"id": 1, "a": '{"x":1,"y":2}'}, {"id": 1, "a": {"y": 2, "x": 1}}], + 0, + False, + ), + ( + "a string spelling an array against the array", + [{"id": 1, "a": "[1,2]"}, {"id": 1, "a": [1, 2]}], + 0, + False, + ), + ( + "equal strings spelling an object", + [{"id": 1, "a": '{"x":1}'}, {"id": 1, "a": '{"x":1}'}], + 1, + True, + ), ] diff --git a/python/tests/test_oracle.py b/python/tests/test_oracle.py new file mode 100644 index 00000000..2f274adc --- /dev/null +++ b/python/tests/test_oracle.py @@ -0,0 +1,275 @@ +"""Profile the oracle-v1 matrix through every input route and check it against +answers computed without dataprof (#829). + +Parity tests compare routes with each other, so they pass when every route is +wrong the same way. `tests/fixtures/oracle-v1.json` holds 20 cases whose +expected values were computed with exact rational arithmetic and 100-digit +decimals, independently of this code: numeric edges (integers past 2^53, the +uint64 boundary, the 64-value SIMD threshold, cancellation, a variance whose +intermediate sum of squares overflows), nested values, and the null +vocabulary. Each case runs through every route that can carry it, and every +disagreement is collected before asserting, so one failing run shows which +routes and fields are wrong. + +`KNOWN` lists the disagreements an open issue owns. It is strict both ways: a +new disagreement fails, and so does a listed one that stops disagreeing, so the +table cannot outlive the fix that empties it. + +`tests/oracle.rs` is the Rust twin for the file engines. +""" + +from __future__ import annotations + +import csv +import hashlib +import io +import json +import math +from collections.abc import Callable, Iterator +from fractions import Fraction +from pathlib import Path +from typing import Any + +import dataprof as dp +import pytest + +pd = pytest.importorskip("pandas") +pa = pytest.importorskip("pyarrow") +pq = pytest.importorskip("pyarrow.parquet") +pl = pytest.importorskip("polars") + +FIXTURE = Path(__file__).parents[2] / "tests/fixtures/oracle-v1.json" +#: The hash published with the fixture on #829, so an edit to an expected +#: value cannot pass unnoticed. +FIXTURE_SHA256 = "fe75498f65bc0b7375d7c0cb09e8c98bbf8999662132f5aff54868a95a495409" +CASES = json.loads(FIXTURE.read_text(encoding="utf-8"))["cases"] + +#: Half a unit of the 4dp serialized precision, plus room for the binary +#: representation of the rounded value. +ABS_TOLERANCE = 0.000051 +#: For magnitudes where 4dp carries no information (around 1e307). +REL_TOLERANCE = 1e-12 + +#: (case, route, field) -> the issue that owns the disagreement. A route of +#: "*" covers every route the case runs on. +KNOWN: dict[tuple[str, str, str], str] = { + # 64-bit integer extrema and order statistics are taken in f64, so values + # past 2^53 collapse: the extremes round, distinct values tie for a mode, + # and the quartiles of a narrow spread coincide. + **{ + (case, "*", field): "#877" + for case in ("above_2_pow_53", "epoch_nanoseconds", "uint64_edge") + for field in ("min", "max", "mode") + }, + ("epoch_nanoseconds", "*", "quartiles.iqr"): "#877", + ("uint64_edge", "*", "quartiles.iqr"): "#877", + # The pandas mixed-object fallback renders a container as the text a + # string can spell, so the two sign one row. + ("container_vs_its_text", "pandas", "duplicate_rows"): "#982", + ("container_and_text_repeats", "pandas", "duplicate_rows"): "#982", +} + +Route = tuple[str, Callable[[], dp.ProfileReport]] +Mismatch = tuple[str, str, str, Any, Any] + + +def _values(case: dict[str, Any]) -> list[Any]: + if case["kind"] != "numeric": + return case["values"] + cast = float if case["dtype"] == "float64" else int + return [cast(value) for value in case["values"]] + + +def _routes(case: dict[str, Any], directory: Path) -> Iterator[Route]: + """Every public route that carries the case's logical data.""" + values = _values(case) + records = [{"v": value} for value in values] + wanted = set(case.get("routes", [])) + + def allowed(route: str) -> bool: + return not wanted or route in wanted + + if allowed("dict"): + yield "dict", lambda: dp.profile({"v": values}) + if allowed("records"): + yield "records", lambda: dp.profile(records) + document = json.dumps(records, allow_nan=False) + if allowed("json"): + path = directory / f"{case['id']}.json" + path.write_text(document, encoding="utf-8") + yield "json", lambda path=path: dp.profile(str(path)) + yield "json_bytes", lambda: dp.profile(document.encode(), format="json") + if allowed("jsonl"): + path = directory / f"{case['id']}.jsonl" + path.write_text("".join(json.dumps(row) + "\n" for row in records), encoding="utf-8") + yield "jsonl", lambda path=path: dp.profile(str(path)) + + # A container in a CSV cell is text, not the same logical value. + if case["kind"] != "nested": + buffer = io.StringIO(newline="") + writer = csv.writer(buffer, quoting=csv.QUOTE_ALL) + writer.writerow(["v"]) + writer.writerows([[value] for value in values]) + text = buffer.getvalue() + path = directory / f"{case['id']}.csv" + path.write_text(text, encoding="utf-8", newline="") + for engine in ("incremental", "columnar"): + yield ( + f"csv_{engine}", + lambda engine=engine, path=path: dp.profile(str(path), engine=engine), + ) + yield "csv_bytes", lambda: dp.profile(text.encode(), format="csv") + + if case["kind"] == "numeric": + dtype = case["dtype"] + frame = pd.DataFrame({"v": pd.Series(values, dtype=dtype)}) + table = pa.table({"v": pa.array(values, type=getattr(pa, dtype)())}) + polars_dtype = {"int64": pl.Int64, "uint64": pl.UInt64, "float64": pl.Float64}[dtype] + polars_frame = pl.DataFrame({"v": pl.Series(values, dtype=polars_dtype)}) + else: + frame = pd.DataFrame({"v": pd.Series(values, dtype=object)}) + table = pa.table({"v": values}) if allowed("arrow") else None + polars_frame = pl.from_arrow(table) if table is not None else None + + if allowed("pandas"): + yield "pandas", lambda: dp.profile(frame) + if table is not None: + if allowed("polars"): + yield "polars", lambda: dp.profile(polars_frame) + if allowed("arrow"): + yield "arrow", lambda: dp.profile(table) + if allowed("parquet"): + path = directory / f"{case['id']}.parquet" + pq.write_table(table, path) + yield "parquet", lambda path=path: dp.profile(str(path)) + + +def _faithful(actual: Any, wanted: str) -> bool: + """Whether `actual` is a faithful binary64 rendering of the exact `wanted`. + + Past 2^53 adjacent floats are hundreds apart, so neither a 4dp nor a + relative tolerance says anything there: the bound is half an ulp of the + exact value, the error rounding to the nearest float may carry. + """ + if actual is None: + return False + exact = Fraction(wanted) + return abs(Fraction(actual) - exact) <= Fraction(math.ulp(float(exact))) / 2 + + +def _close(actual: Any, wanted: str, relative: bool) -> bool: + if actual is None: + return False + target = float(Fraction(wanted)) + if relative: + return math.isclose(float(actual), target, rel_tol=REL_TOLERANCE, abs_tol=0.0) + return math.isclose(float(actual), target, rel_tol=0.0, abs_tol=ABS_TOLERANCE) + + +def _check(case: dict[str, Any], route: str, report: dp.ProfileReport) -> list[Mismatch]: + """Every field of the case's expectation that `report` gets wrong.""" + column = report.to_dict()["columns"][0] + quality = report.to_dict().get("quality") or {} + expected = case.get("expected") or case["expected_common_markers"] + found: list[Mismatch] = [] + + def compare( + field: str, actual: Any, wanted: Any, relative: bool = False, faithful: bool = False + ) -> None: + if wanted is None: + good = actual is None + elif isinstance(wanted, str) and field != "data_type": + good = _faithful(actual, wanted) if faithful else _close(actual, wanted, relative) + else: + good = actual == wanted + if not good: + found.append((case["id"], route, field, wanted, actual)) + + for field in ("total_count", "null_count", "unique_count", "data_type", "stats"): + if field in expected: + compare(field, column.get(field), expected[field]) + if "null_percentage" in expected: + compare("null_percentage", column.get("null_percentage"), expected["null_percentage"]) + if "duplicate_rows_single_column" in expected: + uniqueness = quality.get("uniqueness") or {} + compare( + "duplicate_rows", + uniqueness.get("duplicate_rows"), + expected["duplicate_rows_single_column"], + ) + if case["kind"] != "numeric": + return found + + stats = column.get("stats") or {} + # Past 2^53 an exact integer mean can fall between two binary64 values; + # either neighbour is a faithful float, so the bound is half an ulp. + wide = case["dtype"] != "float64" and any(abs(int(v)) > 2**53 for v in case["values"]) + # Around 1e307 the 4dp rounding carries no information; compare relatively. + extreme = case["id"] == "variance_fits_m2_does_not" + variance = expected["sample_variance_binary64"] if extreme else expected["variance_4dp"] + compare("variance", stats.get("variance"), variance, relative=extreme) + compare("std_dev", stats.get("std_dev"), expected["std_dev_4dp"], relative=extreme) + if wide: + compare("mean", stats.get("mean"), expected["mean_exact"], faithful=True) + else: + compare("mean", stats.get("mean"), expected["mean_4dp"], relative=extreme) + for name in ("min", "max"): + exact = Fraction(expected[f"{name}_exact"]) + actual = stats.get(name) + if wide: + # An exact integer extreme needs an integer-preserving value; + # comparing after float() would erase the defect (#877). + if actual is None or Fraction(actual) != exact: + found.append((case["id"], route, name, str(exact), actual)) + else: + compare(name, actual, str(float(exact)), relative=extreme) + if extreme: + return found + compare("skewness", stats.get("skewness"), expected["adjusted_sample_skewness_4dp"]) + compare("kurtosis", stats.get("kurtosis"), expected["adjusted_sample_excess_kurtosis_4dp"]) + compare("median", stats.get("median"), expected["median_exact"], faithful=wide) + compare("mode", stats.get("mode"), expected["mode_exact"], faithful=wide) + compare("outlier_count", stats.get("outlier_count"), expected["outlier_count"]) + quartiles = stats.get("quartiles") + if expected["quartiles_4dp"] is None: + compare("quartiles", quartiles, None) + else: + for name, value in expected["quartiles_4dp"].items(): + compare(f"quartiles.{name}", (quartiles or {}).get(name), value, faithful=wide) + return found + + +def _owner(mismatch: Mismatch) -> str | None: + case, route, field = mismatch[:3] + return KNOWN.get((case, route, field)) or KNOWN.get((case, "*", field)) + + +def test_the_fixture_is_the_published_one(): + assert hashlib.sha256(FIXTURE.read_bytes()).hexdigest() == FIXTURE_SHA256 + + +@pytest.mark.parametrize("case", CASES, ids=[case["id"] for case in CASES]) +def test_every_route_matches_the_independent_answer(case: dict[str, Any], tmp_path: Path): + seen: list[tuple[str, str]] = [] + mismatches: list[Mismatch] = [] + for route, run in _routes(case, tmp_path): + seen.append((case["id"], route)) + mismatches.extend(_check(case, route, run())) + + unexplained = [m for m in mismatches if _owner(m) is None] + explained = {(m[0], m[1], m[2]) for m in mismatches if _owner(m) is not None} + # A "*" entry claims every route, so each route it covers must still + # disagree: a fix on one route narrows the entry instead of hiding there. + stale = [ + (case["id"], route, key[2]) + for key in KNOWN + if key[0] == case["id"] + for _, route in seen + if key[1] in ("*", route) and (case["id"], route, key[2]) not in explained + ] + report = "\n".join( + f" {route}: {field} expected {wanted!r}, got {actual!r}" + for _, route, field, wanted, actual in unexplained + ) + assert not unexplained, f"{case['id']} disagrees with the oracle:\n{report}" + assert not stale, f"{case['id']}: known disagreements that no longer occur: {stale}" diff --git a/python/tests/test_pandas_mixed_columns.py b/python/tests/test_pandas_mixed_columns.py new file mode 100644 index 00000000..188415e7 --- /dev/null +++ b/python/tests/test_pandas_mixed_columns.py @@ -0,0 +1,209 @@ +"""A pandas object column with mixed Python types must profile cleanly (#976). + +When a pandas DataFrame contains object columns with mixed Python types +(such as numeric columns with placeholder strings, or mixed booleans/dates), +pyarrow cannot cast them into a single physical Arrow type and fails with +``ArrowInvalid`` or ``ArrowTypeError``. + +Rather than aborting the entire profile, such columns are profiled from +their values' text (as ``list[dict]`` input does), with ``type_homogeneity`` +recording the mix, while the other typed columns in the frame keep their +native Arrow-derived profiles unchanged. +""" + +from __future__ import annotations + +import datetime +import decimal + +import dataprof as dp +import pytest + +pd = pytest.importorskip("pandas", reason="pandas is required for pandas DataFrame tests") +pa = pytest.importorskip("pyarrow", reason="pyarrow is required for Arrow import tests") + + +def test_pandas_mixed_object_columns_profile_cleanly(): + """Match list-of-dicts profiles for each mixed-column case from #976.""" + cases = [ + [1, "2", 3], + [1.5, "n/a", 2.0], + [True, "yes", False], + [datetime.date(2024, 1, 1), "unknown", datetime.date(2024, 1, 2)], + ["a", None, "b"], + [1, decimal.Decimal("2.5"), 3], + ] + + for values in cases: + df = pd.DataFrame({"c": pd.Series(values, dtype=object)}) + dataframe_column = dp.profile(df)["c"] + records_column = dp.profile([{"c": value} for value in values])["c"] + + assert dataframe_column.data_type == records_column.data_type + assert dataframe_column.type_homogeneity == records_column.type_homogeneity + + +def test_pandas_mixed_categorical_column_matches_records_route(): + """Mixed categorical values use the same profile as list-of-dicts input.""" + values = [1.5, "n/a", 2.0] + df = pd.DataFrame({"c": pd.Categorical(values)}) + original = df.copy(deep=True) + + dataframe_column = dp.profile(df)["c"] + records_column = dp.profile([{"c": value} for value in values])["c"] + + assert dataframe_column.data_type == records_column.data_type + assert dataframe_column.type_homogeneity == records_column.type_homogeneity + assert df.equals(original) + + +def test_frame_with_mixed_column_and_typed_columns_preserves_typed_profiles(): + """Typed columns keep their native Arrow-derived profiles alongside a mixed column.""" + df_clean = pd.DataFrame( + { + "id": [1, 2, 3], + "score": [95.5, 82.0, 77.5], + "name": ["Alice", "Bob", "Charlie"], + } + ) + clean_report = dp.profile(df_clean) + + df_mixed = pd.DataFrame( + { + "id": [1, 2, 3], + "score": [95.5, 82.0, 77.5], + "name": ["Alice", "Bob", "Charlie"], + "notes": pd.Series([1.5, "n/a", 2.0], dtype=object), + } + ) + mixed_report = dp.profile(df_mixed) + + for col in ("id", "score", "name"): + assert mixed_report[col].data_type == clean_report[col].data_type + assert mixed_report[col].null_count == clean_report[col].null_count + assert mixed_report[col].total_count == clean_report[col].total_count + + assert mixed_report["notes"].data_type == "float" + assert mixed_report["notes"].type_homogeneity == { + "boolean": 0, + "date": 0, + "numeric": 2, + "text": 0, + } + + +def test_unconvertible_column_raises_value_error_naming_column(): + """If a column genuinely cannot be converted, raise ValueError naming the column.""" + + class Unconvertible: + def __str__(self): + raise RuntimeError("cannot stringify object") + + df = pd.DataFrame({"bad_column": [Unconvertible()]}) + with pytest.raises(ValueError, match="column 'bad_column'"): + dp.profile(df) + + +def test_interop_profile_dataframe_mixed_columns(): + """Verify that lower-level interop.profile_dataframe also handles mixed columns.""" + from dataprof.interop import profile_dataframe + + df = pd.DataFrame( + { + "c": pd.Series([1.5, "unknown", 2.0], dtype=object), + } + ) + raw_report = profile_dataframe(df) + report = dp.ProfileReport(raw_report) + assert report["c"].data_type == "string" + assert report["c"].type_homogeneity == { + "boolean": 0, + "date": 0, + "numeric": 2, + "text": 1, + } + + +def test_pandas_multiindex_columns_with_mixed_objects(): + """Verify that DataFrames with tuple/MultiIndex columns and mixed objects profile.""" + df = pd.DataFrame( + { + ("metrics", "score"): pd.Series([1.5, "n/a", 2.0], dtype=object), + ("metrics", "count"): [10, 20, 30], + } + ) + report = dp.profile(df) + score_col = "('metrics', 'score')" + assert report[score_col].data_type == "float" + assert report[score_col].type_homogeneity == { + "boolean": 0, + "date": 0, + "numeric": 2, + "text": 0, + } + + +def test_pandas_duplicate_columns_raises_clear_error(): + """Duplicate column labels fail cleanly and are not misclassified as mixed objects.""" + df = pd.DataFrame([[1, 2]], columns=["a", "a"]) + with pytest.raises(Exception) as exc_info: + dp.profile(df) + assert "Duplicate column names found" in str(exc_info.value) + + +def test_pandas_mixed_columns_preserve_missing_values_as_null(): + """Verify that pd.NA and pd.NaT in mixed object columns become Arrow nulls, + not string literals. + """ + from dataprof._inputs import _pandas_to_pyarrow_table + + df = pd.DataFrame( + { + "id": [10, 20, 30], + "mixed_na": pd.Series([1, "two", pd.NA], dtype=object), + "mixed_nat": pd.Series([1, "two", pd.NaT], dtype=object), + } + ) + original = df.copy(deep=True) + + tbl = _pandas_to_pyarrow_table(df, pa) + assert df.equals(original) + assert tbl["mixed_na"].to_pylist() == ["1", "two", None] + assert tbl["mixed_na"].null_count == 1 + assert tbl["mixed_nat"].to_pylist() == ["1", "two", None] + assert tbl["mixed_nat"].null_count == 1 + + # Neighboring typed column retains native Arrow integer type and values + assert pa.types.is_integer(tbl["id"].type) + assert tbl["id"].to_pylist() == [10, 20, 30] + + report = dp.profile(df) + assert report["mixed_na"].null_count == 1 + assert report["mixed_na"].total_count == 3 + assert report["mixed_na"].type_homogeneity == { + "boolean": 0, + "date": 0, + "numeric": 1, + "text": 1, + } + assert report["mixed_nat"].null_count == 1 + assert report["mixed_nat"].total_count == 3 + assert report["id"].data_type == "integer" + assert report["id"].null_count == 0 + + +def test_pandas_mixed_columns_do_not_misclassify_containers_as_null(): + """Verify container objects like dict and list in mixed columns are not + misclassified as null. + """ + from dataprof._inputs import _pandas_to_pyarrow_table + + df = pd.DataFrame( + { + "mixed_containers": pd.Series([1, "two", {"key": "val"}, [1, 2]], dtype=object), + } + ) + + tbl = _pandas_to_pyarrow_table(df, pa) + assert tbl["mixed_containers"].to_pylist() == ["1", "two", '{"key":"val"}', "[1,2]"] + assert tbl["mixed_containers"].null_count == 0 diff --git a/python/tests/test_profiler_configuration.py b/python/tests/test_profiler_configuration.py index eda1cd82..c5001bb9 100644 --- a/python/tests/test_profiler_configuration.py +++ b/python/tests/test_profiler_configuration.py @@ -298,6 +298,7 @@ def test_returns_self(self): assert p.positive_columns(["pressure"]) is p assert p.identifier_columns(["order_id"]) is p assert p.temporal_columns(["observed_on"]) is p + assert p.temporal_pairs([("valid_from", "valid_until")]) is p def test_csv_delimiter(self): if not os.path.exists(SEMICOLON_FILE): diff --git a/python/tests/test_python_api.py b/python/tests/test_python_api.py index cb53bbf3..97b8affc 100644 --- a/python/tests/test_python_api.py +++ b/python/tests/test_python_api.py @@ -702,6 +702,7 @@ def test_profile_signature(self): "positive_columns", "identifier_columns", "temporal_columns", + "temporal_pairs", } actual_params = set(sig.parameters.keys()) assert actual_params == expected_params, ( diff --git a/python/tests/test_report_delta.py b/python/tests/test_report_delta.py new file mode 100644 index 00000000..4e2b6870 --- /dev/null +++ b/python/tests/test_report_delta.py @@ -0,0 +1,154 @@ +"""ProfileReport.compare() returns the Rust report delta (#713). + +`tests/report_delta.rs` is the Rust twin. Here the extra routes are the +Python ones: a report loaded from a flat `to_dict()` summary, including a +summary written by 0.11, compares through the same delta as a profiled one. +""" + +import json +from pathlib import Path + +import dataprof as dp + +FIXTURES = Path(__file__).parents[2] / "tests/fixtures" +CLEAN = {"id": [1, 2, 3, 4], "amount": [10, 20, 30, 40]} +WITH_NULLS = {"id": [1, 2, 3, 4], "amount": [10, None, None, 40]} +SCORE_KEYS = {"a", "b", "abs", "rel_pct", "evidence"} + + +def _flat(report: dp.ProfileReport) -> dp.ProfileReport: + return dp.ProfileReport.from_dict(report.to_dict()) + + +def test_the_shape_is_the_frozen_contract(): + delta = dp.profile(WITH_NULLS).compare(dp.profile(CLEAN)) + assert list(delta) == [ + "quality_score", + "dimensions", + "columns", + "metric_semantics", + "schema", + "reports", + ] + assert set(delta["quality_score"]) == SCORE_KEYS + assert list(delta["dimensions"]) == [ + "completeness", + "consistency", + "uniqueness", + "accuracy", + "timeliness", + "validity", + "precision", + ] + for change in delta["dimensions"].values(): + assert set(change) == SCORE_KEYS + assert set(change["evidence"]) == {"a", "b"} + assert delta["columns"]["amount"] == { + "null_pct_a": 50.0, + "null_pct_b": 0.0, + "null_pct_delta": -50.0, + } + assert set(delta["metric_semantics"]) == {"a", "b", "comparable"} + assert set(delta["schema"]) == {"added", "removed", "common"} + for side in delta["reports"].values(): + assert set(side) == {"id", "timestamp", "source", "rows_processed", "evidence"} + + +def test_every_change_is_b_minus_a(): + delta = dp.profile(WITH_NULLS).compare(dp.profile(CLEAN)) + completeness = delta["dimensions"]["completeness"] + assert completeness["b"] > completeness["a"] + assert completeness["abs"] == round(completeness["b"] - completeness["a"], 2) + assert completeness["abs"] > 0 + assert delta["columns"]["amount"]["null_pct_delta"] < 0 + + +def test_a_flat_summary_compares_like_the_report_it_came_from(): + a, b = dp.profile(WITH_NULLS), dp.profile(CLEAN) + native = a.compare(b) + flat = _flat(a).compare(_flat(b)) + mixed = _flat(a).compare(b) + + assert native["reports"]["a"]["id"] == json.loads(a.to_json())["id"] + for delta in (flat, mixed): + assert delta["reports"]["a"]["id"] is None + assert delta["reports"]["a"]["timestamp"] is None + masked = json.loads(json.dumps(native)) + for side in ("a", "b"): + masked["reports"][side]["id"] = None + masked["reports"][side]["timestamp"] = None + assert flat == masked + masked["reports"]["b"] = native["reports"]["b"] + assert mixed == masked + + +def test_a_0_11_baseline_reads_as_unrecorded(tmp_path: Path): + text = (FIXTURES / "legacy_0_11_non_ascii.summary.json").read_text(encoding="utf-8") + baseline = dp.ProfileReport.from_dict(json.loads(text)) + path = tmp_path / "cities.csv" + path.write_text("city\n東京\nZürich\nOslo\n\n", encoding="utf-8") + current = dp.profile(str(path)) + delta = baseline.compare(current) + + assert delta["metric_semantics"]["comparable"] is None + # 0.11 did not record which quality components were sampled. + assert delta["quality_score"]["evidence"]["a"] == { + "coverage": "incomplete", + "reason": "coverage_unrecorded", + } + assert delta["quality_score"]["evidence"]["b"] == {"coverage": "complete"} + assert delta["reports"]["a"]["evidence"] == {"coverage": "complete"} + + +def test_a_truncated_side_says_so(tmp_path: Path): + path = tmp_path / "rows.csv" + path.write_text("id,amount\n" + "".join(f"{i},{i * 2}\n" for i in range(200))) + exact = dp.profile(str(path)) + truncated = dp.profile(str(path), max_rows=50) + delta = exact.compare(truncated) + + truncated_evidence = {"coverage": "incomplete", "reason": "truncated"} + assert delta["reports"]["a"]["evidence"] == {"coverage": "complete"} + assert delta["reports"]["b"]["evidence"] == truncated_evidence + assert delta["reports"]["b"]["rows_processed"] == 50 + assert delta["quality_score"]["evidence"]["b"] == truncated_evidence + for change in delta["dimensions"].values(): + assert change["evidence"]["b"] == truncated_evidence + + +def test_nothing_assessed_invents_no_number(): + empty = dp.profile({"id": [], "amount": []}) + for delta in (empty.compare(dp.profile(CLEAN)), empty.compare(empty)): + assert delta["quality_score"]["abs"] is None + assert delta["quality_score"]["rel_pct"] is None + assert all(change["abs"] is None for change in delta["dimensions"].values()) + assert all(change["null_pct_delta"] is None for change in delta["columns"].values()) + + +def test_a_summary_that_records_only_its_row_count_still_compares(): + """The oldest accepted flat summary records `rows_processed` and nothing + else about its scan, so its coverage is unrecorded, never complete.""" + legacy = dp.ProfileReport.from_dict( + {"source": "old.csv", "columns": [], "execution": {"rows_processed": 5}} + ) + unrecorded = {"coverage": "incomplete", "reason": "coverage_unrecorded"} + for delta in (legacy.compare(legacy), legacy.compare(dp.profile(CLEAN))): + assert delta["reports"]["a"]["rows_processed"] == 5 + assert delta["reports"]["a"]["evidence"] == unrecorded + assert delta["quality_score"]["abs"] is None + + +def test_a_repeated_column_name_reads_its_first_profile_as_check_does(): + report = dp.profile(CLEAN) + document = json.loads(report.to_json()) + first, second = dict(document["column_profiles"][1]), dict(document["column_profiles"][1]) + first["null_count"] = 0 + second["null_count"] = second["total_count"] + document["column_profiles"] = [document["column_profiles"][0], first, second] + rebuilt = dp.ProfileReport.from_dict(document) + + delta = rebuilt.compare(rebuilt) + assert delta["schema"]["common"] == ["id", "amount"] + assert delta["columns"]["amount"]["null_pct_a"] == 0.0 + observed = rebuilt.check(max_null_percentage={"amount": 50}).checks + assert [c.observed for c in observed if c.column == "amount"] == [0.0] diff --git a/python/tests/test_report_exports.py b/python/tests/test_report_exports.py index 8065bb24..c744d809 100644 --- a/python/tests/test_report_exports.py +++ b/python/tests/test_report_exports.py @@ -410,6 +410,199 @@ def test_save_accepts_pathlike(self, report, tmp_path): assert result is report assert dataprof.ProfileReport.load(path).to_dict() == report.to_dict() + def test_failed_json_save_preserves_existing_file(self, report, tmp_path, monkeypatch): + path = tmp_path / "report.json" + original = b"previous good report\n" + path.write_bytes(original) + + def fail_to_json(): + raise RuntimeError("simulated serialization failure") + + monkeypatch.setattr(report, "to_json", fail_to_json) + + with pytest.raises(RuntimeError, match="simulated serialization failure"): + report.save(path) + + assert path.read_bytes() == original + + def test_failed_json_write_preserves_existing_file_and_cleans_temp( + self, report, tmp_path, monkeypatch + ): + import builtins + + path = tmp_path / "report.json" + original = b"previous good report\n" + path.write_bytes(original) + + real_open = builtins.open + + class FailingFile: + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + return False + + def write(self, data): + raise OSError("simulated write failure") + + def failing_open(file, *args, **kwargs): + name = Path(file).name + + if name.startswith(".report.json.") and name.endswith(".tmp"): + return FailingFile() + + return real_open(file, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", failing_open) + + with pytest.raises(OSError, match="simulated write failure"): + report.save(path) + + monkeypatch.undo() + + assert path.read_bytes() == original + assert list(tmp_path.glob(".report.json.*.tmp")) == [] + + @pytest.mark.parametrize( + ("suffix", "method_name"), + [ + (".json", "to_json"), + (".csv", "_records"), + (".parquet", "to_arrow"), + (".html", "to_html"), + (".md", "to_markdown"), + (".markdown", "to_markdown"), + ], + ) + def test_failed_serialization_preserves_existing_file( + self, report, tmp_path, monkeypatch, suffix, method_name + ): + path = tmp_path / f"report{suffix}" + original = b"previous good report\n" + path.write_bytes(original) + + def fail(): + raise RuntimeError("simulated serialization failure") + + monkeypatch.setattr(report, method_name, fail) + + with pytest.raises(RuntimeError, match="simulated serialization failure"): + report.save(path) + + assert path.read_bytes() == original + + @pytest.mark.parametrize( + "suffix", + [".csv", ".html", ".md", ".markdown"], + ) + def test_failed_write_preserves_existing_file_and_cleans_temp( + self, report, tmp_path, monkeypatch, suffix + ): + import builtins + + path = tmp_path / f"report{suffix}" + original = b"previous good report\n" + path.write_bytes(original) + + real_open = builtins.open + + class FailingFile: + def __init__(self, inner): + self.inner = inner + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.inner.close() + return False + + def write(self, data): + # Attempt to simulate a real partial write before the failure. + partial = data[: max(1, len(data) // 2)] + self.inner.write(partial) + self.inner.flush() + raise OSError("simulated write failure") + + def failing_open(file, *args, **kwargs): + name = Path(file).name + + if name.startswith(f".report{suffix}.") and name.endswith(".tmp"): + return FailingFile(real_open(file, *args, **kwargs)) + + return real_open(file, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", failing_open) + + with pytest.raises(OSError, match="simulated write failure"): + report.save(path) + + monkeypatch.undo() + + assert path.read_bytes() == original + assert list(tmp_path.glob(f".report{suffix}.*.tmp")) == [] + + @pytest.mark.skipif( + os.name == "nt", + reason="POSIX file permissions are not available on Windows", + ) + def test_save_preserves_existing_file_permissions(self, report, tmp_path): + path = tmp_path / "report.json" + path.write_text("old report", encoding="utf-8") + path.chmod(0o640) + + report.save(path) + + assert path.stat().st_mode & 0o777 == 0o640 + + def test_new_save_uses_default_file_permissions(self, report, tmp_path): + reference = tmp_path / "reference.txt" + reference.write_text("reference", encoding="utf-8") + + path = tmp_path / "report.json" + report.save(path) + + assert path.stat().st_mode & 0o777 == reference.stat().st_mode & 0o777 + + def test_save_through_symlink_preserves_symlink(self, report, tmp_path): + target = tmp_path / "target.json" + target.write_text("old report", encoding="utf-8") + + path = tmp_path / "report.json" + + try: + path.symlink_to(target) + except OSError as exc: + pytest.skip(f"symlinks are not available: {exc}") + + report.save(path) + + assert path.is_symlink() + assert dataprof.ProfileReport.load(path).to_dict() == report.to_dict() + + def test_failed_parquet_write_preserves_existing_file_and_cleans_temp( + self, report, tmp_path, monkeypatch + ): + pq = pytest.importorskip("pyarrow.parquet") + + path = tmp_path / "report.parquet" + original = b"previous good report\n" + path.write_bytes(original) + + def failing_write_table(table, destination, *args, **kwargs): + temp_path = Path(destination) + temp_path.write_bytes(b"partial parquet data") + raise OSError("simulated parquet write failure") + + monkeypatch.setattr(pq, "write_table", failing_write_table) + + with pytest.raises(OSError, match="simulated parquet write failure"): + report.save(path) + + assert path.read_bytes() == original + assert list(tmp_path.glob(".report.parquet.*.tmp")) == [] + def test_save_empty_report_csv_creates_artifact(self, tmp_path): report = dataprof.profile({}) path = tmp_path / "empty.csv" @@ -424,18 +617,17 @@ def test_save_and_load_extensions_are_case_insensitive(self, report, tmp_path): assert report.save(path) is report assert dataprof.ProfileReport.load(path).to_dict() == report.to_dict() - def test_save_parquet(self, report): + def test_save_parquet(self, report, tmp_path): pytest.importorskip("pyarrow") pq = pytest.importorskip("pyarrow.parquet") - with tempfile.NamedTemporaryFile(suffix=".parquet", delete=False) as f: - path = f.name - try: - result = report.save(path) - assert result is report - table = pq.read_table(path) - assert table.num_rows == report.columns - finally: - os.unlink(path) + + path = tmp_path / "report.parquet" + + result = report.save(path) + + assert result is report + table = pq.read_table(path) + assert table.num_rows == report.columns def test_to_json_is_byte_stable_for_unchanged_input(tmp_path): diff --git a/python/tests/test_timeliness_declared_pairs.py b/python/tests/test_timeliness_declared_pairs.py new file mode 100644 index 00000000..7337575b --- /dev/null +++ b/python/tests/test_timeliness_declared_pairs.py @@ -0,0 +1,200 @@ +"""Start/end pairs declared next to ``temporal_columns`` (#942). + +Timeliness found pairs only from role words in the names, so +``valid_from``/``valid_until``, ``checkin``/``checkout`` or names in another +language were never compared. A declared pair is compared as given on every +engine and input path, and an unknown name in one is reported like any other +hint. + +``tests/timeliness_declared_pairs.rs`` is the Rust twin. +""" + +from __future__ import annotations + +import datetime as dt +import importlib.util +import json +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import pytest + +try: + import dataprof as dp +except ImportError: + pytest.skip( + "dataprof native extension not built. Run: maturin develop", + allow_module_level=True, + ) + +START, END = "valid_from", "valid_until" +#: The start of every row. +STARTS = ["2024-03-15", "2024-04-20", "2024-05-10"] +#: The end, before the start on every row, so the pair counts three violations. +ENDS = ["2024-02-01", "2024-03-01", "2024-03-20"] + +Route = Callable[..., Any] + + +def _dates(values: list[str]) -> list[dt.date]: + return [dt.date.fromisoformat(value) for value in values] + + +def _records() -> list[dict[str, str]]: + return [{START: start, END: end} for start, end in zip(STARTS, ENDS, strict=True)] + + +def _csv(tmp_path: Path) -> Path: + path = tmp_path / "validity.csv" + rows = [f"{START},{END}", *(f"{a},{b}" for a, b in zip(STARTS, ENDS, strict=True))] + path.write_text("\n".join(rows) + "\n", encoding="utf-8") + return path + + +def _routes(tmp_path: Path) -> dict[str, Route]: + """Every input route, each profiling the two date columns with ``**hints``.""" + + def csv_file(engine: str) -> Route: + return lambda **hints: dp.profile(str(_csv(tmp_path)), engine=engine, **hints) + + def json_file(**hints: Any) -> Any: + path = tmp_path / "validity.json" + path.write_text(json.dumps(_records()), encoding="utf-8") + return dp.profile(str(path), **hints) + + def jsonl_file(**hints: Any) -> Any: + path = tmp_path / "validity.jsonl" + path.write_text("\n".join(json.dumps(r) for r in _records()) + "\n", encoding="utf-8") + return dp.profile(str(path), **hints) + + def arrow_table() -> Any: + import pyarrow as pa + + return pa.table( + { + START: pa.array(_dates(STARTS), pa.date32()), + END: pa.array(_dates(ENDS), pa.date32()), + } + ) + + def parquet_file(**hints: Any) -> Any: + import pyarrow.parquet as pq + + path = tmp_path / "validity.parquet" + pq.write_table(arrow_table(), path) + return dp.profile(str(path), **hints) + + def arrow(**hints: Any) -> Any: + return dp.profile(arrow_table(), **hints) + + def pandas(**hints: Any) -> Any: + import pandas as pd + + return dp.profile(pd.DataFrame({START: STARTS, END: ENDS}), **hints) + + def polars(**hints: Any) -> Any: + import polars as pl + + return dp.profile(pl.DataFrame({START: STARTS, END: ENDS}), **hints) + + def builder(**hints: Any) -> Any: + profiler = dp.Profiler().temporal_pairs(hints["temporal_pairs"]) + return profiler.profile(str(_csv(tmp_path))) + + routes: dict[str, Route] = { + "auto": csv_file("auto"), + "incremental": csv_file("incremental"), + "columnar": csv_file("columnar"), + "json": json_file, + "jsonl": jsonl_file, + "dict": lambda **hints: dp.profile({START: STARTS, END: ENDS}, **hints), + "rows": lambda **hints: dp.profile(_records(), **hints), + "profiler": builder, + } + # An optional library drops only its own routes; skipping inside a route + # would skip the whole test and every route it had not reached yet. + optional = {"parquet": ("pyarrow", parquet_file), "arrow": ("pyarrow", arrow)} + optional |= {"pandas": ("pandas", pandas), "polars": ("polars", polars)} + for label, (module, route) in optional.items(): + if importlib.util.find_spec(module) is not None: + routes[label] = route + return routes + + +def _ordering(report: Any) -> tuple[int, int]: + timeliness = report.to_dict()["quality"]["timeliness"] + return timeliness["temporal_pairs_checked"], timeliness["temporal_violations"] + + +def test_a_declared_pair_is_compared_on_every_path(tmp_path: Path): + wrong = [] + for label, route in _routes(tmp_path).items(): + for pairs, expected in (([], (0, 0)), ([(START, END)], (3, 3))): + report = route(temporal_pairs=pairs) + found = _ordering(report) + if found != expected: + wrong.append(f"[{label}] declared {pairs}: {found}, expected {expected}") + # Each pair column binds once, as the option the caller used. + bound = [ + (binding["column"], binding["matched_values"]) + for binding in report.semantic_hint_bindings + if binding["kind"] == "temporal_pair" + ] + expected_bound = [(START, 3), (END, 3)] if pairs else [] + if bound != expected_bound: + wrong.append( + f"[{label}] declared {pairs}: bindings {bound}, expected {expected_bound}" + ) + assert not wrong, "\n".join(wrong) + + +def test_a_declared_pair_over_columns_without_dates_is_rejected(tmp_path: Path): + # The clean check that never happened: two text columns used to profile + # fine and report ``temporal_pairs_checked == 0``. + columns = {"name": ["Ada", "Linus", "Grace"], "city": ["Turin", "Helsinki", "Arlington"]} + path = tmp_path / "people.csv" + rows = [ + "name,city", + *(f"{n},{c}" for n, c in zip(columns["name"], columns["city"], strict=True)), + ] + path.write_text("\n".join(rows) + "\n", encoding="utf-8") + sources: dict[str, Any] = { + engine: (str(path), {"engine": engine}) for engine in ("auto", "incremental", "columnar") + } + sources["dict"] = (columns, {}) + if importlib.util.find_spec("pyarrow") is not None: + import pyarrow as pa + + sources["arrow"] = (pa.table(columns), {}) + for label, (source, options) in sources.items(): + with pytest.raises(ValueError) as caught: + dp.profile(source, temporal_pairs=[("name", "city")], **options) + message = str(caught.value) + assert "'name' (temporal_pairs)" in message, label + assert "'city' (temporal_pairs)" in message, label + + +def test_a_pair_given_as_a_list_works_like_a_tuple(tmp_path: Path): + report = dp.profile(str(_csv(tmp_path)), temporal_pairs=[[START, END]]) # ty: ignore[invalid-argument-type] + assert _ordering(report) == (3, 3) + + +def test_an_unknown_column_in_a_declared_pair_is_rejected(tmp_path: Path): + for label, route in _routes(tmp_path).items(): + with pytest.raises(ValueError) as caught: + route(temporal_pairs=[(START, "valid_untill")]) + message = str(caught.value) + assert "'valid_untill'" in message, label + assert "temporal_pairs" in message, label + + +@pytest.mark.parametrize("pair", [[START], [START, END, END]]) +def test_an_entry_that_is_not_two_names_is_rejected(tmp_path: Path, pair: list[str]): + with pytest.raises(ValueError, match="temporal_pairs entries must be"): + dp.profile(str(_csv(tmp_path)), temporal_pairs=[pair]) # ty: ignore[invalid-argument-type] + + +def test_config_reports_the_declared_pairs(): + config = dp.ProfilerConfig(temporal_pairs=[(START, END)]) + assert config.temporal_pairs == [(START, END)] diff --git a/python/tests/test_type_homogeneity.py b/python/tests/test_type_homogeneity.py index 053495c4..1e682aad 100644 --- a/python/tests/test_type_homogeneity.py +++ b/python/tests/test_type_homogeneity.py @@ -163,9 +163,10 @@ def test_the_published_schema_rejects_what_the_loader_discards(self): "a partial count map must not validate, because the loader drops it" ) - def test_a_malformed_mapping_reads_as_absence(self): + def test_a_malformed_mapping_is_refused(self): # An invented count is worse than a gap: a partial mapping cannot be # completed with zeros, because zero is a claim about the data. + # It is refused with ValueError naming the field path (#950). document = { "schema_version": dp.REPORT_SCHEMA_VERSION, "source": "hand-edited.csv", @@ -183,7 +184,8 @@ def test_a_malformed_mapping_reads_as_absence(self): "quality": None, } - assert dp.ProfileReport.from_dict(document)["v"].type_homogeneity is None + with pytest.raises(ValueError, match=r"'columns\[0\]\.type_homogeneity' must be"): + dp.ProfileReport.from_dict(document) def test_the_flat_exports_carry_the_derived_scalars(self, tmp_path): # to_dataframe()/save(".csv") are flat and cannot hold the count map, so diff --git a/tests/all_null_columns.rs b/tests/all_null_columns.rs new file mode 100644 index 00000000..83e16fd1 --- /dev/null +++ b/tests/all_null_columns.rs @@ -0,0 +1,112 @@ +//! A column with no non-null value carries no statistics (#983). +//! +//! Statistics are aggregates over values, and over none they are undefined, +//! which the report states as absent. An all-null text column used to report +//! lengths of 0, the same as a column of real zero-length values, while an +//! all-null numeric column already reported no statistics. +//! +//! `python/tests/test_all_null_columns.py` is the Python twin, over the +//! in-memory routes. + +use std::io::Write; + +use dataprof::{ColumnStats, EngineType, ProfileReport, Profiler}; + +const ENGINES: [EngineType; 3] = [ + EngineType::Auto, + EngineType::Incremental, + EngineType::Columnar, +]; + +fn write(suffix: &str, contents: &str) -> tempfile::NamedTempFile { + let mut file = tempfile::Builder::new().suffix(suffix).tempfile().unwrap(); + write!(file, "{contents}").unwrap(); + file.flush().unwrap(); + file +} + +/// Every route this file can reach, by label. +fn reports( + csv: &str, + jsonl: &str, + profiler: impl Fn() -> Profiler, +) -> Vec<(String, ProfileReport)> { + let csv_file = write(".csv", csv); + let jsonl_file = write(".jsonl", jsonl); + let mut out: Vec<(String, ProfileReport)> = ENGINES + .iter() + .map(|engine| { + ( + format!("csv {engine:?}"), + profiler() + .engine(*engine) + .analyze_file(csv_file.path()) + .unwrap(), + ) + }) + .collect(); + out.push(( + "jsonl".to_string(), + profiler().analyze_file(jsonl_file.path()).unwrap(), + )); + out +} + +fn column<'r>(report: &'r ProfileReport, name: &str) -> &'r dataprof::ColumnProfile { + report + .column_profiles + .iter() + .find(|column| column.name == name) + .unwrap() +} + +#[test] +fn a_column_of_nulls_reports_no_statistics_and_keeps_its_counts() { + // `blank` holds only null tokens; `note` holds one real value. + let csv = "id,empty,blank,note\n1,,NA,\n2,,null,x\n3,,,\n"; + let jsonl = concat!( + "{\"id\":1,\"empty\":null,\"blank\":\"NA\",\"note\":null}\n", + "{\"id\":2,\"empty\":null,\"blank\":\"null\",\"note\":\"x\"}\n", + "{\"id\":3,\"empty\":null,\"blank\":\"\",\"note\":null}\n", + ); + let mut wrong = Vec::new(); + for (route, report) in reports(csv, jsonl, Profiler::new) { + for name in ["empty", "blank"] { + let profile = column(&report, name); + if !matches!(profile.stats, ColumnStats::None) { + wrong.push(format!("{route}: {name} stats {:?}", profile.stats)); + } + if (profile.total_count, profile.null_count) != (3, 3) { + wrong.push(format!( + "{route}: {name} counts {}/{}", + profile.null_count, profile.total_count + )); + } + } + // One value is enough for lengths. + match &column(&report, "note").stats { + ColumnStats::Text(text) if (text.min_length, text.max_length) == (1, 1) => {} + other => wrong.push(format!("{route}: note stats {other:?}")), + } + } + assert!(wrong.is_empty(), "{}", wrong.join("\n")); +} + +#[test] +fn an_identifier_column_of_nulls_reports_no_statistics() { + let csv = "id,code\n1,\n2,\n"; + let jsonl = "{\"id\":1,\"code\":null}\n{\"id\":2,\"code\":null}\n"; + let mut wrong = Vec::new(); + for (route, report) in reports(csv, jsonl, || { + Profiler::new().identifier_columns(vec!["code".to_string()]) + }) { + let profile = column(&report, "code"); + if !matches!(profile.stats, ColumnStats::None) { + wrong.push(format!( + "{route}: {:?} {:?}", + profile.data_type, profile.stats + )); + } + } + assert!(wrong.is_empty(), "{}", wrong.join("\n")); +} diff --git a/tests/async_streaming.rs b/tests/async_streaming.rs index 70a77454..a458d8a0 100644 --- a/tests/async_streaming.rs +++ b/tests/async_streaming.rs @@ -545,3 +545,277 @@ async fn test_profiler_profile_stream_finished_event_counts_bytes_stop_condition "finished event total_rows must match execution.rows_processed" ); } + +/// A progress channel that is full when profiling ends must still deliver the +/// `Finished` event to a consumer draining the channel (gh #974). +#[tokio::test(flavor = "current_thread")] +async fn test_progress_channel_capacity_one_receives_finished_event() { + for stop_cond in [ + None, + Some(StopCondition::MaxRows(150)), + Some(StopCondition::MaxBytes(50_000)), + ] { + let mut csv_data = String::from("id,value\n"); + for i in 0..10_000 { + csv_data.push_str(&format!("{},val_{}\n", i, i)); + } + + let source = BytesSource::new( + bytes::Bytes::from(csv_data.clone()), + AsyncSourceInfo::new("bytes-csv", FileFormat::Csv) + .size_hint(Some(csv_data.len() as u64)), + ); + + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let consumer_task = tokio::spawn(async move { + let mut finished_received = false; + while let Some(event) = rx.recv().await { + if let dataprof::ProgressEvent::Finished { .. } = event { + finished_received = true; + break; + } + } + finished_received + }); + + let mut profiler = Profiler::new() + .progress_interval(Duration::ZERO) + .progress_channel(tx); + + if let Some(ref cond) = stop_cond { + profiler = profiler.stop_when(cond.clone()); + } + + let report = profiler.profile_stream(source).await.unwrap(); + assert!(report.execution.rows_processed > 0); + + drop(profiler); + + let finished_received = consumer_task.await.unwrap(); + assert!( + finished_received, + "Finished event must be received by channel consumer for stop_condition {:?}", + stop_cond + ); + } +} + +/// A stream that fails during profiling must never emit `ProgressEvent::Finished`. +#[tokio::test] +async fn test_progress_channel_does_not_emit_finished_on_error() { + // 1. Duplicate CSV headers error + { + let csv = b"x,x\n1,2\n"; + let source = BytesSource::new( + bytes::Bytes::from_static(csv), + AsyncSourceInfo::new("duplicate-header-csv", FileFormat::Csv), + ); + + let (tx, mut rx) = tokio::sync::mpsc::channel(10); + let consumer = tokio::spawn(async move { + let mut got_finished = false; + while let Some(event) = rx.recv().await { + if let dataprof::ProgressEvent::Finished { .. } = event { + got_finished = true; + } + } + got_finished + }); + + let res = Profiler::new() + .progress_channel(tx) + .profile_stream(source) + .await; + + assert!( + res.is_err(), + "profiling should fail due to duplicate headers" + ); + let got_finished = consumer.await.unwrap(); + assert!( + !got_finished, + "Finished event must not be emitted when profiling fails on duplicate headers" + ); + } + + // 2. Unterminated quote error in strict CSV mode + { + let csv = b"id,val\n1,\"unterminated\n2,normal\n"; + let source = BytesSource::new( + bytes::Bytes::from_static(csv), + AsyncSourceInfo::new("unterminated-quote-csv", FileFormat::Csv), + ); + + let (tx, mut rx) = tokio::sync::mpsc::channel(10); + let consumer = tokio::spawn(async move { + let mut got_finished = false; + while let Some(event) = rx.recv().await { + if let dataprof::ProgressEvent::Finished { .. } = event { + got_finished = true; + } + } + got_finished + }); + + let res = Profiler::new() + .csv_flexible(false) + .progress_channel(tx) + .profile_stream(source) + .await; + + assert!( + res.is_err(), + "profiling should fail due to unterminated quote in strict mode" + ); + let got_finished = consumer.await.unwrap(); + assert!( + !got_finished, + "Finished event must not be emitted when profiling fails on unterminated quote" + ); + } +} + +/// A progress channel whose capacity is 1 must deliver `Finished` without blocking +/// or timing out even when the caller drains the channel only after profiling completes (gh #974). +#[tokio::test] +async fn test_progress_channel_drained_after_profile_under_timeout() { + let mut csv_data = String::from("id,value\n"); + for i in 0..10_000 { + csv_data.push_str(&format!("{},val_{}\n", i, i)); + } + + let source = BytesSource::new( + bytes::Bytes::from(csv_data.clone()), + AsyncSourceInfo::new("drained-after-csv", FileFormat::Csv) + .size_hint(Some(csv_data.len() as u64)), + ); + + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let profiler = Profiler::new() + .progress_interval(Duration::ZERO) + .progress_channel(tx); + + let report = tokio::time::timeout(Duration::from_secs(2), profiler.profile_stream(source)) + .await + .expect("profiling must not deadlock or time out while channel is not yet drained") + .unwrap(); + + assert!(report.execution.rows_processed > 0); + + let mut finished_received = false; + while let Ok(event) = rx.try_recv() { + if let dataprof::ProgressEvent::Finished { .. } = event { + finished_received = true; + break; + } + } + assert!( + finished_received, + "Finished event must be received by consumer draining after profiling completes" + ); +} + +/// A facade-bracketed columnar file route must also deliver `Finished` using the reserved permit. +#[test] +fn test_progress_channel_file_route_receives_finished_event() { + use std::io::Write; + + let mut tmp = tempfile::NamedTempFile::with_suffix(".csv").unwrap(); + writeln!(tmp, "id,value").unwrap(); + for i in 0..100 { + writeln!(tmp, "{},val_{}", i, i).unwrap(); + } + tmp.flush().unwrap(); + + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let report = Profiler::new() + .engine(dataprof::EngineType::Columnar) + .progress_interval(Duration::ZERO) + .progress_channel(tx) + .analyze_file(tmp.path()) + .unwrap(); + + assert_eq!(report.execution.rows_processed, 100); + + let mut finished_received = false; + while let Ok(event) = rx.try_recv() { + if let dataprof::ProgressEvent::Finished { .. } = event { + finished_received = true; + break; + } + } + assert!( + finished_received, + "Finished event must be delivered on file routes via reserved permit" + ); +} + +/// Reusing a profiler must reserve fresh terminal capacity for every run. +#[test] +fn test_progress_channel_reused_profiler_receives_finished_each_run() { + use std::io::Write; + + let mut tmp = tempfile::NamedTempFile::with_suffix(".csv").unwrap(); + writeln!(tmp, "id,value").unwrap(); + for i in 0..100 { + writeln!(tmp, "{},val_{}", i, i).unwrap(); + } + tmp.flush().unwrap(); + + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let profiler = Profiler::new() + .progress_interval(Duration::ZERO) + .progress_channel(tx); + + for run in 1..=2 { + let report = profiler.analyze_file(tmp.path()).unwrap(); + assert_eq!(report.execution.rows_processed, 100); + + let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); + assert!( + events + .iter() + .any(|event| matches!(event, dataprof::ProgressEvent::Finished { .. })), + "run {run} must receive a freshly reserved Finished event; got {events:?}" + ); + } +} + +/// Reusing a facade-bracketed file route must reserve fresh terminal capacity for every run. +#[test] +fn test_progress_channel_reused_columnar_file_route_receives_finished_each_run() { + use std::io::Write; + + let mut tmp = tempfile::NamedTempFile::with_suffix(".csv").unwrap(); + writeln!(tmp, "id,value").unwrap(); + for i in 0..100 { + writeln!(tmp, "{},val_{}", i, i).unwrap(); + } + tmp.flush().unwrap(); + + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let profiler = Profiler::new() + .engine(dataprof::EngineType::Columnar) + .progress_interval(Duration::ZERO) + .progress_channel(tx); + + for run in 1..=2 { + let report = profiler.analyze_file(tmp.path()).unwrap(); + assert_eq!(report.execution.rows_processed, 100); + + let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); + assert!( + events + .iter() + .any(|event| matches!(event, dataprof::ProgressEvent::Finished { .. })), + "columnar run {run} must receive a freshly reserved Finished event; got {events:?}" + ); + } +} + +/// Preserve the public enum constructor accepted before gh #974. +#[test] +fn test_progress_sink_channel_variant_accepts_sender() { + let (tx, _rx) = tokio::sync::mpsc::channel(1); + let _sink = dataprof::ProgressSink::Channel(tx); +} diff --git a/tests/cross_engine_consistency.rs b/tests/cross_engine_consistency.rs index 9b4b0969..badbe3bc 100644 --- a/tests/cross_engine_consistency.rs +++ b/tests/cross_engine_consistency.rs @@ -34,9 +34,18 @@ fn serialized_text_stats_match_column_analysis_and_csv_engines() { let standard = analyze_csv_file(csv.path(), &CsvParserConfig::default()).unwrap(); let expected = serde_json::to_value(&standard.column_profiles[0].stats).unwrap(); - assert!(expected["Text"].is_object(), "{values:?}"); - assert!(expected["Text"].get("most_frequent").is_none()); - assert!(expected["Text"].get("least_frequent").is_none()); + // A column of nulls only has no values, so no statistics (#983); the + // others are text, with frequencies left unassessed. + if values + .iter() + .all(|value| dataprof::is_null_like_token(value.trim())) + { + assert_eq!(expected, json!("None"), "{values:?}"); + } else { + assert!(expected["Text"].is_object(), "{values:?}"); + assert!(expected["Text"].get("most_frequent").is_none()); + assert!(expected["Text"].get("least_frequent").is_none()); + } for column in [ dataprof::analyze_column("text", &data), diff --git a/tests/fixtures/oracle-v1.json b/tests/fixtures/oracle-v1.json new file mode 100644 index 00000000..d9b1c646 --- /dev/null +++ b/tests/fixtures/oracle-v1.json @@ -0,0 +1,938 @@ +{ + "fixture_version": 1, + "audited_commit": "0d8d6ad0aa7d343334b487349076bca96b97d83f", + "oracle": "fractions.Fraction + 100-digit decimal.Decimal; no dataprof imports", + "conventions": { + "variance": "sample, ddof=1", + "quartiles": "Hyndman-Fan type 7; absent below four observations", + "mode": "absent when all unique; smallest tied modal value", + "numeric_strings": "parse according to dtype before constructing typed inputs", + "undefined": "null, never zero except the documented constant shape convention", + "float_input": "exact rational value of supplied IEEE binary64", + "precision": "4dp ordinary statistics, 2dp percentages; exact integer targets remain strings" + }, + "cases": [ + { + "id": "ordinary", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "1", + "2", + "3", + "4" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 0, + "min_exact": "1", + "max_exact": "4", + "mean_exact": "5/2", + "sample_variance_exact": "5/3", + "mean_binary64": "2.5", + "sample_variance_binary64": "1.6666666666666667", + "mean_4dp": "2.5000", + "variance_4dp": "1.6667", + "std_dev_4dp": "1.2910", + "median_exact": "5/2", + "mode_exact": null, + "quartiles_4dp": { + "q1": "1.7500", + "q2": "2.5000", + "q3": "3.2500", + "iqr": "1.5000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "-1.2000" + }, + "notes": [] + }, + { + "id": "mixed_sign_zero_mean", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "-10", + "-2", + "0", + "4", + "8" + ], + "expected": { + "total_count": 5, + "null_count": 0, + "unique_count": 5, + "duplicate_rows_single_column": 0, + "min_exact": "-10", + "max_exact": "8", + "mean_exact": "0", + "sample_variance_exact": "46", + "mean_binary64": "0.0", + "sample_variance_binary64": "46.0", + "mean_4dp": "0.0000", + "variance_4dp": "46.0000", + "std_dev_4dp": "6.7823", + "median_exact": "0", + "mode_exact": null, + "quartiles_4dp": { + "q1": "-2.0000", + "q2": "0.0000", + "q3": "4.0000", + "iqr": "6.0000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "-0.5769", + "adjusted_sample_excess_kurtosis_4dp": "0.4877" + }, + "notes": [] + }, + { + "id": "constant", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "7", + "7", + "7", + "7" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "unique_count": 1, + "duplicate_rows_single_column": 3, + "min_exact": "7", + "max_exact": "7", + "mean_exact": "7", + "sample_variance_exact": "0", + "mean_binary64": "7.0", + "sample_variance_binary64": "0.0", + "mean_4dp": "7.0000", + "variance_4dp": "0.0000", + "std_dev_4dp": "0.0000", + "median_exact": "7", + "mode_exact": "7", + "quartiles_4dp": { + "q1": "7.0000", + "q2": "7.0000", + "q3": "7.0000", + "iqr": "0.0000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "0.0000" + }, + "notes": [ + "Zero shape moments are dataprof's explicit constant-column convention; mathematically standardized moments are undefined." + ] + }, + { + "id": "two_values", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "1", + "2" + ], + "expected": { + "total_count": 2, + "null_count": 0, + "unique_count": 2, + "duplicate_rows_single_column": 0, + "min_exact": "1", + "max_exact": "2", + "mean_exact": "3/2", + "sample_variance_exact": "1/2", + "mean_binary64": "1.5", + "sample_variance_binary64": "0.5", + "mean_4dp": "1.5000", + "variance_4dp": "0.5000", + "std_dev_4dp": "0.7071", + "median_exact": "3/2", + "mode_exact": null, + "quartiles_4dp": null, + "outlier_count": null, + "adjusted_sample_skewness_4dp": null, + "adjusted_sample_excess_kurtosis_4dp": null + }, + "notes": [] + }, + { + "id": "asymmetric", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "1", + "2", + "3", + "10", + "4", + "5" + ], + "expected": { + "total_count": 6, + "null_count": 0, + "unique_count": 6, + "duplicate_rows_single_column": 0, + "min_exact": "1", + "max_exact": "10", + "mean_exact": "25/6", + "sample_variance_exact": "61/6", + "mean_binary64": "4.166666666666667", + "sample_variance_binary64": "10.166666666666666", + "mean_4dp": "4.1667", + "variance_4dp": "10.1667", + "std_dev_4dp": "3.1885", + "median_exact": "7/2", + "mode_exact": null, + "quartiles_4dp": { + "q1": "2.2500", + "q2": "3.5000", + "q3": "4.7500", + "iqr": "2.5000" + }, + "outlier_count": 1, + "adjusted_sample_skewness_4dp": "1.4396", + "adjusted_sample_excess_kurtosis_4dp": "2.4377" + }, + "notes": [] + }, + { + "id": "above_2_pow_53", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "9007199254740993", + "9007199254740995", + "9007199254740997" + ], + "expected": { + "total_count": 3, + "null_count": 0, + "unique_count": 3, + "duplicate_rows_single_column": 0, + "min_exact": "9007199254740993", + "max_exact": "9007199254740997", + "mean_exact": "9007199254740995", + "sample_variance_exact": "4", + "mean_binary64": "9007199254740996.0", + "sample_variance_binary64": "4.0", + "mean_4dp": "9007199254740995.0000", + "variance_4dp": "4.0000", + "std_dev_4dp": "2.0000", + "median_exact": "9007199254740995", + "mode_exact": null, + "quartiles_4dp": null, + "outlier_count": null, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": null + }, + "notes": [ + "#877: exact extrema/order statistics remain known limitations. Mean may be nearest binary64; do not demand an unrepresentable exact mean from a float field." + ] + }, + { + "id": "epoch_nanoseconds", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "1759700000123456789", + "1759700000123456791", + "1759700000123456795", + "1759700000123456800" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 0, + "min_exact": "1759700000123456789", + "max_exact": "1759700000123456800", + "mean_exact": "7038800000493827175/4", + "sample_variance_exact": "283/12", + "mean_binary64": "1.7597000001234568e+18", + "sample_variance_binary64": "23.583333333333332", + "mean_4dp": "1759700000123456793.7500", + "variance_4dp": "23.5833", + "std_dev_4dp": "4.8563", + "median_exact": "1759700000123456793", + "mode_exact": null, + "quartiles_4dp": { + "q1": "1759700000123456790.5000", + "q2": "1759700000123456793.0000", + "q3": "1759700000123456796.2500", + "iqr": "5.7500" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.6876", + "adjusted_sample_excess_kurtosis_4dp": "-0.9465" + }, + "notes": [ + "#877: extrema, mode and IQR are expected failures; variance and shape are mandatory now." + ] + }, + { + "id": "uint64_edge", + "kind": "numeric", + "column": "v", + "dtype": "uint64", + "values": [ + "18446744073709551612", + "18446744073709551613", + "18446744073709551614", + "18446744073709551615" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 0, + "min_exact": "18446744073709551612", + "max_exact": "18446744073709551615", + "mean_exact": "36893488147419103227/2", + "sample_variance_exact": "5/3", + "mean_binary64": "1.8446744073709552e+19", + "sample_variance_binary64": "1.6666666666666667", + "mean_4dp": "18446744073709551613.5000", + "variance_4dp": "1.6667", + "std_dev_4dp": "1.2910", + "median_exact": "36893488147419103227/2", + "mode_exact": null, + "quartiles_4dp": { + "q1": "18446744073709551612.7500", + "q2": "18446744073709551613.5000", + "q3": "18446744073709551614.2500", + "iqr": "1.5000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "-1.2000" + }, + "notes": [ + "Preserve as uint64/decimal strings; never build a pandas float64 series first. #877 order-statistic limitation applies." + ] + }, + { + "id": "simd_64", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3" + ], + "expected": { + "total_count": 64, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 60, + "min_exact": "0", + "max_exact": "3", + "mean_exact": "3/2", + "sample_variance_exact": "80/63", + "mean_binary64": "1.5", + "sample_variance_binary64": "1.2698412698412698", + "mean_4dp": "1.5000", + "variance_4dp": "1.2698", + "std_dev_4dp": "1.1269", + "median_exact": "3/2", + "mode_exact": "0", + "quartiles_4dp": { + "q1": "0.7500", + "q2": "1.5000", + "q3": "2.2500", + "iqr": "1.5000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "-1.3726" + }, + "notes": [] + }, + { + "id": "simd_65", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0", + "1", + "2", + "3", + "0" + ], + "expected": { + "total_count": 65, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 61, + "min_exact": "0", + "max_exact": "3", + "mean_exact": "96/65", + "sample_variance_exact": "167/130", + "mean_binary64": "1.476923076923077", + "sample_variance_binary64": "1.2846153846153847", + "mean_4dp": "1.4769", + "variance_4dp": "1.2846", + "std_dev_4dp": "1.1334", + "median_exact": "1", + "mode_exact": "0", + "quartiles_4dp": { + "q1": "0.0000", + "q2": "1.0000", + "q3": "2.0000", + "iqr": "2.0000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0257", + "adjusted_sample_excess_kurtosis_4dp": "-1.3883" + }, + "notes": [] + }, + { + "id": "large_offset_64", + "kind": "numeric", + "column": "v", + "dtype": "int64", + "values": [ + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003", + "1000000000000", + "1000000000001", + "1000000000002", + "1000000000003" + ], + "expected": { + "total_count": 64, + "null_count": 0, + "unique_count": 4, + "duplicate_rows_single_column": 60, + "min_exact": "1000000000000", + "max_exact": "1000000000003", + "mean_exact": "2000000000003/2", + "sample_variance_exact": "80/63", + "mean_binary64": "1000000000001.5", + "sample_variance_binary64": "1.2698412698412698", + "mean_4dp": "1000000000001.5000", + "variance_4dp": "1.2698", + "std_dev_4dp": "1.1269", + "median_exact": "2000000000003/2", + "mode_exact": "1000000000000", + "quartiles_4dp": { + "q1": "1000000000000.7500", + "q2": "1000000000001.5000", + "q3": "1000000000002.2500", + "iqr": "1.5000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "-1.3726" + }, + "notes": [] + }, + { + "id": "cancellation", + "kind": "numeric", + "column": "v", + "dtype": "float64", + "values": [ + "-1000000000000.0", + "1.0", + "1000000000000.0" + ], + "expected": { + "total_count": 3, + "null_count": 0, + "unique_count": 3, + "duplicate_rows_single_column": 0, + "min_exact": "-1000000000000", + "max_exact": "1000000000000", + "mean_exact": "1/3", + "sample_variance_exact": "3000000000000000000000001/3", + "mean_binary64": "0.3333333333333333", + "sample_variance_binary64": "1e+24", + "mean_4dp": "0.3333", + "variance_4dp": "1000000000000000000000000.3333", + "std_dev_4dp": "1000000000000.0000", + "median_exact": "1", + "mode_exact": null, + "quartiles_4dp": null, + "outlier_count": null, + "adjusted_sample_skewness_4dp": "-0.0000", + "adjusted_sample_excess_kurtosis_4dp": null + }, + "notes": [] + }, + { + "id": "variance_fits_m2_does_not", + "kind": "numeric", + "column": "v", + "dtype": "float64", + "values": [ + "-7e+153", + "7e+153", + "-7e+153", + "7e+153" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "unique_count": 2, + "duplicate_rows_single_column": 2, + "min_exact": "-6999999999999999812063107609186712233361878314499896009468692980217164324595475341466780559922101737600776669044728186587542627454761660556073094092947456", + "max_exact": "6999999999999999812063107609186712233361878314499896009468692980217164324595475341466780559922101737600776669044728186587542627454761660556073094092947456", + "mean_exact": "0", + "sample_variance_exact": "195999999999999989475534026114456026349367271676528028779069413094081833701602300025803936559401972368386088681890464802872128308312907921015659788480645324324307119799838030887033365081000266559657524758864226094648549510952190699937329163951435954071576762323756507534959814884274963111659533347830307487744/3", + "mean_binary64": "0.0", + "sample_variance_binary64": "6.533333333333333e+307", + "mean_4dp": "0.0000", + "variance_4dp": "65333333333333329825178008704818675449789090558842676259689804364693944567200766675267978853133990790000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.0000", + "std_dev_4dp": "8082903768654760486117252102031840939217375263803738313097158955132786283991214063878752744551741607000000000000000000000000000000000000000000000000000000.0000", + "median_exact": "0", + "mode_exact": "-6999999999999999812063107609186712233361878314499896009468692980217164324595475341466780559922101737600776669044728186587542627454761660556073094092947456", + "quartiles_4dp": { + "q1": "-6999999999999999812063107609186712233361878314499896009468692980217164324595475341466780559922101738000000000000000000000000000000000000000000000000000000.0000", + "q2": "0.0000", + "q3": "6999999999999999812063107609186712233361878314499896009468692980217164324595475341466780559922101738000000000000000000000000000000000000000000000000000000.0000", + "iqr": "13999999999999999624126215218373424466723756628999792018937385960434328649190950682933561119844203480000000000000000000000000000000000000000000000000000000.0000" + }, + "outlier_count": 0, + "adjusted_sample_skewness_4dp": "0.0000", + "adjusted_sample_excess_kurtosis_4dp": "-6.0000" + }, + "notes": [ + "Use sample_variance_binary64, relative tolerance 1e-12; a fixed decimal-place tolerance is meaningless at this magnitude. Check not-null. Higher moments/outlier fences are outside this first extreme-float assertion set." + ] + }, + { + "id": "object_key_order", + "kind": "nested", + "column": "v", + "values": [ + { + "a": 1, + "b": 2 + }, + { + "b": 2, + "a": 1 + }, + { + "a": 2, + "b": 2 + }, + null + ], + "routes": [ + "dict", + "records", + "json", + "jsonl", + "pandas", + "arrow", + "parquet", + "polars" + ], + "expected": { + "total_count": 4, + "null_count": 1, + "duplicate_rows_single_column": 1, + "data_type": "nested", + "stats": null, + "unique_count": null + } + }, + { + "id": "list_order", + "kind": "nested", + "column": "v", + "values": [ + [ + 1, + 2 + ], + [ + 2, + 1 + ], + [ + 1, + 2 + ], + [], + null + ], + "routes": [ + "dict", + "records", + "json", + "jsonl", + "pandas", + "arrow", + "parquet", + "polars" + ], + "expected": { + "total_count": 5, + "null_count": 1, + "duplicate_rows_single_column": 1, + "data_type": "nested", + "stats": null, + "unique_count": null + } + }, + { + "id": "inner_null_is_not_outer_null", + "kind": "nested", + "column": "v", + "values": [ + { + "x": null + }, + { + "x": 1 + }, + null, + { + "x": null + } + ], + "routes": [ + "dict", + "records", + "json", + "jsonl", + "pandas", + "arrow", + "parquet", + "polars" + ], + "expected": { + "total_count": 4, + "null_count": 1, + "duplicate_rows_single_column": 1, + "data_type": "nested", + "stats": null, + "unique_count": null + } + }, + { + "id": "container_vs_its_text", + "kind": "nested", + "column": "v", + "values": [ + { + "x": 1 + }, + "{\"x\":1}", + [ + 1, + 2 + ], + "[1,2]" + ], + "routes": [ + "dict", + "records", + "json", + "jsonl", + "pandas" + ], + "expected": { + "total_count": 4, + "null_count": 0, + "duplicate_rows_single_column": 0 + } + }, + { + "id": "container_and_text_repeats", + "kind": "nested", + "column": "v", + "values": [ + { + "x": 1 + }, + "{\"x\":1}", + [ + 1, + 2 + ], + "[1,2]", + { + "x": 1 + }, + "{\"x\":1}" + ], + "routes": [ + "dict", + "records", + "json", + "jsonl", + "pandas" + ], + "expected": { + "total_count": 6, + "null_count": 0, + "duplicate_rows_single_column": 2 + } + }, + { + "id": "null_tokens_and_near_misses", + "kind": "nulls", + "column": "v", + "values": [ + null, + "", + " ", + "\t", + "null", + "NULL", + "NaN", + "NA", + "N/A", + "n/a", + "#N/A", + "\\N", + "None", + "Na", + "na", + "none", + "NONE", + "#n/a", + "\\n", + "N/a", + "-", + "?", + "n.d.", + "0" + ], + "expected_common_markers": { + "total_count": 24, + "null_count": 13, + "null_percentage": "54.17", + "unique_count": 11, + "duplicate_rows_single_column": 1 + }, + "expected_typed_only": { + "null_count": 1, + "null_percentage": "4.17" + }, + "notes": [ + "Duplicate-row identity is raw lexical identity, not equality after the null vocabulary: only None and the empty string collide here. The other null-like spellings remain different row signatures by the documented RowSignature contract.", + "Typed-only mode is a proposed #846 setting, not an option on this commit. CSV carries an explicit sentinel for typed-only nulls; quoted empty fields alone are not a universal null protocol." + ] + }, + { + "id": "all_null", + "kind": "nulls", + "column": "v", + "values": [ + null, + null, + null, + null + ], + "expected_common_markers": { + "total_count": 4, + "null_count": 4, + "null_percentage": "100.00", + "unique_count": 0, + "stats": null, + "duplicate_rows_single_column": 3 + }, + "notes": [ + "The absence of text extrema/mean is the mathematical target. Master returns empty TextStats with zero lengths: record as an explicit absence-contract decision rather than silently changing the expected answer to match implementation." + ] + } + ] +} diff --git a/tests/nested_duplicate_rows.rs b/tests/nested_duplicate_rows.rs index ce604b22..719d0e7d 100644 --- a/tests/nested_duplicate_rows.rs +++ b/tests/nested_duplicate_rows.rs @@ -5,6 +5,8 @@ //! different order were not duplicates. Arrow and Parquet rendered a nested //! value as its display string, which does not quote strings or show a null //! list element, so `["p, q"]` matched `["p", "q"]` and `[null]` matched `[]`. +//! A container's text is its compact JSON, which a string can spell, so a +//! string equal to it signed the same row as the container (#945). //! //! `python/tests/test_nested_duplicate_rows.py` is the Python twin. @@ -22,7 +24,7 @@ struct Case { arrow: bool, } -const CASES: [Case; 6] = [ +const CASES: [Case; 9] = [ Case { name: "key order", records: r#"{"id": 1, "a": {"x": 1, "y": 2}} @@ -67,6 +69,28 @@ const CASES: [Case; 6] = [ duplicates: 0, arrow: false, }, + // #945: a container crosses as its compact JSON, which a string can spell. + Case { + name: "a string spelling an object against the object", + records: r#"{"id": 1, "a": "{\"x\":1,\"y\":2}"} +{"id": 1, "a": {"y": 2, "x": 1}}"#, + duplicates: 0, + arrow: false, + }, + Case { + name: "a string spelling an array against the array", + records: r#"{"id": 1, "a": "[1,2]"} +{"id": 1, "a": [1, 2]}"#, + duplicates: 0, + arrow: false, + }, + Case { + name: "equal strings spelling an object", + records: r#"{"id": 1, "a": "{\"x\":1}"} +{"id": 1, "a": "{\"x\":1}"}"#, + duplicates: 1, + arrow: true, + }, ]; fn file(suffix: &str, contents: &[u8]) -> NamedTempFile { diff --git a/tests/oracle.rs b/tests/oracle.rs new file mode 100644 index 00000000..edfcedca --- /dev/null +++ b/tests/oracle.rs @@ -0,0 +1,472 @@ +//! Profile the oracle-v1 matrix through the file engines and check it against +//! answers computed without dataprof (#829). +//! +//! `tests/fixtures/oracle-v1.json` holds 20 cases whose expected values were +//! computed with exact rational arithmetic, independently of this code. This +//! runs each case as CSV through the incremental and columnar engines and as +//! JSON and JSONL, compares at the serialized precision, and collects every +//! disagreement before asserting. `KNOWN` lists the ones an open issue owns, +//! strictly: a listed disagreement that stops occurring fails too. +//! +//! `python/tests/test_oracle.py` is the Python twin, and adds the in-memory +//! routes (dicts, records, pandas, polars, Arrow) and Parquet. + +use std::path::Path; + +use dataprof::{EngineType, ProfileReport, Profiler}; +use serde_json::{Value, json}; + +const ROUTES: [&str; 4] = ["csv_incremental", "csv_columnar", "json", "jsonl"]; + +/// Half a unit of the 4dp serialized precision, plus room for the binary +/// representation of the rounded value. +const ABS_TOLERANCE: f64 = 0.000051; +/// For magnitudes where 4dp carries no information (around 1e307). +const REL_TOLERANCE: f64 = 1e-12; + +/// (case, route, field) and the issue that owns the disagreement. A route of +/// "*" covers every route the case runs on. The same entries as the Python +/// twin, for the routes this file runs. +const KNOWN: &[(&str, &str, &str, &str)] = &[ + // 64-bit integer extrema and order statistics are taken in f64, so values + // past 2^53 collapse: the extremes round, distinct values tie for a mode, + // and the quartiles of a narrow spread coincide. + ("above_2_pow_53", "*", "min", "#877"), + ("above_2_pow_53", "*", "max", "#877"), + ("above_2_pow_53", "*", "mode", "#877"), + ("epoch_nanoseconds", "*", "min", "#877"), + ("epoch_nanoseconds", "*", "max", "#877"), + ("epoch_nanoseconds", "*", "mode", "#877"), + ("epoch_nanoseconds", "*", "quartiles.iqr", "#877"), + ("uint64_edge", "*", "min", "#877"), + ("uint64_edge", "*", "max", "#877"), + ("uint64_edge", "*", "mode", "#877"), + ("uint64_edge", "*", "quartiles.iqr", "#877"), +]; + +#[derive(Debug, Clone, PartialEq)] +struct Mismatch { + case: String, + route: &'static str, + field: String, + expected: String, + actual: String, +} + +fn fixture() -> Value { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/oracle-v1.json"); + serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap() +} + +/// The case's values as JSON, typed by its dtype: numeric values are strings +/// in the fixture so no JSON tool can round them before profiling. +fn values(case: &Value) -> Vec { + let raw = case["values"].as_array().unwrap(); + if case["kind"] != "numeric" { + return raw.clone(); + } + raw.iter() + .map(|value| { + let text = value.as_str().unwrap(); + // serde_json keeps an integer exact up to u64; a float parses as f64. + serde_json::from_str::(text).unwrap_or_else(|_| json!(text)) + }) + .collect() +} + +fn csv_text(values: &[Value]) -> String { + let mut text = String::from("\"v\"\n"); + for value in values { + let cell = match value { + Value::Null => String::new(), + Value::String(text) => text.replace('"', "\"\""), + // Python's csv writer prints a float with repr; the fixture's + // float strings are already that form. + other => other.to_string(), + }; + text.push_str(&format!("\"{cell}\"\n")); + } + text +} + +fn profile(case: &Value, route: &str, dir: &Path) -> Option { + let values = values(case); + let records: Vec = values.iter().map(|value| json!({"v": value})).collect(); + let id = case["id"].as_str().unwrap(); + let wanted = case["routes"].as_array(); + let allowed = |name: &str| wanted.is_none_or(|routes| routes.iter().any(|r| r == name)); + let path = match route { + // A container in a CSV cell is text, not the same logical value. + "csv_incremental" | "csv_columnar" if case["kind"] == "nested" => return None, + "csv_incremental" | "csv_columnar" => { + let path = dir.join(format!("{id}.csv")); + std::fs::write(&path, csv_text(&values)).unwrap(); + path + } + "json" if allowed("json") => { + let path = dir.join(format!("{id}.json")); + std::fs::write(&path, serde_json::to_string(&records).unwrap()).unwrap(); + path + } + "jsonl" if allowed("jsonl") => { + let path = dir.join(format!("{id}.jsonl")); + let lines: String = records.iter().map(|row| format!("{row}\n")).collect(); + std::fs::write(&path, lines).unwrap(); + path + } + _ => return None, + }; + let profiler = match route { + "csv_incremental" => Profiler::new().engine(EngineType::Incremental), + "csv_columnar" => Profiler::new().engine(EngineType::Columnar), + _ => Profiler::new(), + }; + Some( + profiler + .analyze_file(&path) + .unwrap_or_else(|e| panic!("{id} via {route}: {e}")), + ) +} + +/// A fraction such as `"5/3"`, or a decimal, as the nearest f64. +fn number(text: &str) -> f64 { + match text.split_once('/') { + Some((num, den)) => num.parse::().unwrap() / den.parse::().unwrap(), + None => text.parse().unwrap(), + } +} + +/// An exact rational over i128, for integer cases past 2^53: an integer, a +/// fraction such as `"36893488147419103227/2"`, or a decimal such as +/// `"18446744073709551612.7500"`. +fn rational(text: &str) -> (i128, i128) { + if let Some((num, den)) = text.split_once('/') { + return (num.parse().unwrap(), den.parse().unwrap()); + } + match text.split_once('.') { + Some((whole, fraction)) => ( + format!("{whole}{fraction}").parse().unwrap(), + 10_i128.pow(fraction.len() as u32), + ), + None => (text.parse().unwrap(), 1), + } +} + +/// A reported number as an exact integer when it is one. A JSON integer is +/// read as an integer: `as_f64` would round `9007199254740993` to a +/// neighbour, and a fixed #877 would then still read as wrong. +fn exact_integer(value: &Value) -> Option { + value + .as_i64() + .map(i128::from) + .or_else(|| value.as_u64().map(i128::from)) + .or_else(|| { + value + .as_f64() + .filter(|f| f.fract() == 0.0 && f.abs() < 1e38) + .map(|f| f as i128) + }) +} + +/// Whether `actual` is a faithful binary64 rendering of the exact `wanted`. +/// +/// Past 2^53 adjacent floats are hundreds apart, so neither a 4dp nor a +/// relative tolerance says anything there: the bound is half an ulp of the +/// exact value, the error rounding to the nearest float may carry. +fn faithful(actual: &Value, wanted: &str) -> bool { + let (num, den) = rational(wanted); + let target = num as f64 / den as f64; + match exact_integer(actual) { + Some(a) => (a * den - num).abs() * 2 <= (ulp(target) as i128) * den, + None => actual + .as_f64() + .is_some_and(|a| (a - target).abs() <= ulp(target)), + } +} + +/// Collects the fields of one (case, route) run that disagree with the oracle. +struct Checker<'a> { + case: &'a str, + route: &'static str, + found: Vec, +} + +impl Checker<'_> { + fn record(&mut self, field: &str, wanted: impl ToString, actual: &Value) { + self.found.push(Mismatch { + case: self.case.to_string(), + route: self.route, + field: field.to_string(), + expected: wanted.to_string(), + actual: actual.to_string(), + }); + } + + /// Compare at the serialized precision; `relative` for magnitudes where + /// 4dp carries no information. + fn compare(&mut self, field: &str, actual: &Value, wanted: &Value, relative: bool) { + let good = match wanted { + Value::Null => actual.is_null(), + Value::String(text) if field != "data_type" => actual.as_f64().is_some_and(|a| { + let target = number(text); + if relative { + (a - target).abs() <= REL_TOLERANCE * target.abs() + } else { + (a - target).abs() <= ABS_TOLERANCE + } + }), + other => actual == other, + }; + if !good { + self.record(field, wanted, actual); + } + } + + /// Compare against an exact value within half an ulp; for values past + /// 2^53, where 4dp and relative tolerances say nothing. + fn compare_faithful(&mut self, field: &str, actual: &Value, wanted: &Value) { + let good = match wanted { + Value::Null => actual.is_null(), + Value::String(text) => faithful(actual, text), + other => actual == other, + }; + if !good { + self.record(field, wanted, actual); + } + } +} + +/// The gap from `value` to the next f64 away from zero. +fn ulp(value: f64) -> f64 { + let magnitude = value.abs(); + f64::from_bits(magnitude.to_bits() + 1) - magnitude +} + +fn check(case: &Value, route: &'static str, report: &ProfileReport) -> Vec { + let summary: Value = serde_json::from_str(&report.summary_json().unwrap()).unwrap(); + let column = &summary["columns"][0]; + let expected = if case["expected"].is_object() { + &case["expected"] + } else { + &case["expected_common_markers"] + }; + let id = case["id"].as_str().unwrap(); + let mut checker = Checker { + case: id, + route, + found: Vec::new(), + }; + + for field in [ + "total_count", + "null_count", + "unique_count", + "data_type", + "stats", + "null_percentage", + ] { + if let Some(wanted) = expected.get(field) { + checker.compare(field, &column[field], wanted, false); + } + } + if let Some(wanted) = expected.get("duplicate_rows_single_column") { + checker.compare( + "duplicate_rows", + &summary["quality"]["uniqueness"]["duplicate_rows"], + wanted, + false, + ); + } + if case["kind"] != "numeric" { + return checker.found; + } + + let stats = &column["stats"]; + let wide = case["dtype"] != "float64" + && case["values"] + .as_array() + .unwrap() + .iter() + .any(|v| rational(v.as_str().unwrap()).0.unsigned_abs() > 1 << 53); + let extreme = id == "variance_fits_m2_does_not"; + let variance = if extreme { + &expected["sample_variance_binary64"] + } else { + &expected["variance_4dp"] + }; + checker.compare("variance", &stats["variance"], variance, extreme); + checker.compare( + "std_dev", + &stats["std_dev"], + &expected["std_dev_4dp"], + extreme, + ); + + if wide { + // Past 2^53 every f64 is an integer, and an exact mean can fall between + // two of them; either neighbour is faithful, so the bound is half an ulp. + checker.compare_faithful("mean", &stats["mean"], &expected["mean_exact"]); + // An exact integer extreme needs an integer-preserving value; + // comparing after a float conversion would erase the defect (#877). + for name in ["min", "max"] { + let wanted = expected[format!("{name}_exact")].as_str().unwrap(); + let actual = &stats[name]; + if exact_integer(actual) != Some(rational(wanted).0) { + checker.record(name, wanted, actual); + } + } + } else { + checker.compare("mean", &stats["mean"], &expected["mean_4dp"], extreme); + for name in ["min", "max"] { + let exact = number(expected[format!("{name}_exact")].as_str().unwrap()); + checker.compare(name, &stats[name], &json!(exact.to_string()), extreme); + } + } + if extreme { + return checker.found; + } + checker.compare( + "skewness", + &stats["skewness"], + &expected["adjusted_sample_skewness_4dp"], + false, + ); + checker.compare( + "kurtosis", + &stats["kurtosis"], + &expected["adjusted_sample_excess_kurtosis_4dp"], + false, + ); + let mode = stats.get("mode").cloned().unwrap_or(Value::Null); + if wide { + checker.compare_faithful("median", &stats["median"], &expected["median_exact"]); + checker.compare_faithful("mode", &mode, &expected["mode_exact"]); + } else { + checker.compare("median", &stats["median"], &expected["median_exact"], false); + checker.compare("mode", &mode, &expected["mode_exact"], false); + } + checker.compare( + "outlier_count", + &stats["outlier_count"], + &expected["outlier_count"], + false, + ); + match expected["quartiles_4dp"].as_object() { + None => checker.compare( + "quartiles", + stats.get("quartiles").unwrap_or(&Value::Null), + &Value::Null, + false, + ), + Some(quartiles) => { + for (name, wanted) in quartiles { + let field = format!("quartiles.{name}"); + let actual = &stats["quartiles"][name]; + if wide { + checker.compare_faithful(&field, actual, wanted); + } else { + checker.compare(&field, actual, wanted, false); + } + } + } + } + checker.found +} + +fn owner(m: &Mismatch) -> Option<&'static str> { + KNOWN + .iter() + .find(|(case, route, field, _)| { + *case == m.case && (*route == "*" || *route == m.route) && *field == m.field + }) + .map(|(.., issue)| *issue) +} + +#[test] +fn every_file_engine_matches_the_independent_answer() { + let dir = tempfile::tempdir().unwrap(); + let mut runs = Vec::new(); + let mut mismatches = Vec::new(); + for case in fixture()["cases"].as_array().unwrap() { + for route in ROUTES { + if let Some(report) = profile(case, route, dir.path()) { + runs.push((case["id"].as_str().unwrap().to_string(), route)); + mismatches.extend(check(case, route, &report)); + } + } + } + + let unexplained: Vec = mismatches + .iter() + .filter(|m| owner(m).is_none()) + .map(|m| { + format!( + " {} via {}: {} expected {}, got {}", + m.case, m.route, m.field, m.expected, m.actual + ) + }) + .collect(); + // A "*" entry claims every route, so each route it covers must still + // disagree: a fix on one route narrows the entry instead of hiding there. + let stale: Vec = KNOWN + .iter() + .flat_map(|(case, route, field, issue)| { + runs.iter() + .filter(move |(c, r)| c == case && (*route == "*" || route == r)) + .filter(|(c, r)| { + !mismatches + .iter() + .any(|m| m.case == *c && m.route == *r && m.field == *field) + }) + .map(move |(_, r)| format!(" {case} via {r}: {field} ({issue})")) + }) + .collect(); + + assert!( + unexplained.is_empty(), + "disagreements with the oracle:\n{}", + unexplained.join("\n") + ); + assert!( + stale.is_empty(), + "known disagreements that no longer occur:\n{}", + stale.join("\n") + ); + // Every case ran through at least one route. + assert_eq!( + runs.iter() + .map(|(case, _)| case.as_str()) + .collect::>() + .len(), + 20 + ); +} + +/// The comparison helpers for values past 2^53, on inputs the current data +/// cannot reach: an exact JSON integer (what a fixed #877 would report), and a +/// median a million away from the truth. +#[test] +fn the_wide_value_helpers_are_exact() { + let exact = json!(9_007_199_254_740_993_u64); + assert_eq!(exact_integer(&exact), Some(9_007_199_254_740_993)); + assert!(faithful(&exact, "9007199254740993")); + // Its nearest float is a faithful rendering too; a neighbour two ulps off is not. + assert!(faithful( + &json!(9_007_199_254_740_992.0), + "9007199254740993" + )); + assert!(!faithful( + &json!(9_007_199_254_740_996.0), + "9007199254740993" + )); + + let median = "1759700000123456794"; + assert!(faithful(&json!(1.759_700_000_123_456_8e18), median)); + assert!(!faithful(&json!(1.759_700_000_123_456_8e18 + 1e6), median)); + + assert_eq!( + rational("18446744073709551612.7500"), + (184_467_440_737_095_516_127_500, 10_000) + ); + assert_eq!(rational("-5/3"), (-5, 3)); +} diff --git a/tests/public_api_facade.rs b/tests/public_api_facade.rs index e461cb89..f325d0a2 100644 --- a/tests/public_api_facade.rs +++ b/tests/public_api_facade.rs @@ -623,3 +623,98 @@ fn quality_gate_types_are_matched_with_a_wildcard() { let empty = QualityPolicy::new().evaluate(&report).unwrap_err(); assert_eq!(policy_error(&empty), 1); } + +/// The report-delta types are `#[non_exhaustive]` so #749 and #310 can add +/// fields without breaking callers. This names every field through the facade +/// in code that must compile, so a rename fails here. +#[test] +fn report_delta_fields_are_reachable_through_the_facade() { + use dataprof::{ + ColumnChange, DimensionChange, ReportDelta, ReportReference, SchemaChange, ScoreChange, + SemanticsComparison, Sides, + }; + + fn score(change: &ScoreChange) -> usize { + let ScoreChange { + a, + b, + abs, + rel_pct, + evidence: Sides { + a: left, b: right, .. + }, + .. + } = change; + let _ = (a, b, abs, rel_pct, left, right); + 1 + } + + fn reference(side: &ReportReference) -> usize { + let ReportReference { + id, + timestamp, + source, + rows_processed, + evidence, + .. + } = side; + let _ = (id, timestamp, evidence); + source.len() + rows_processed.unwrap_or(0) + } + + fn delta(delta: &ReportDelta) -> usize { + let ReportDelta { + quality_score, + dimensions, + columns, + metric_semantics: + SemanticsComparison { + a: semantics_a, + b: semantics_b, + comparable, + .. + }, + schema: + SchemaChange { + added, + removed, + common, + .. + }, + reports: Sides { a, b, .. }, + .. + } = delta; + let _ = (semantics_a, semantics_b, comparable, added, removed, common); + let dimension_total: usize = dimensions + .iter() + .map( + |DimensionChange { + dimension, + score: change, + .. + }| { + let _ = dimension; + score(change) + }, + ) + .sum(); + let column_total: usize = columns + .iter() + .map( + |ColumnChange { + name, + null_pct_a, + null_pct_b, + null_pct_delta, + .. + }| { + let _ = (null_pct_a, null_pct_b, null_pct_delta); + name.len() + }, + ) + .sum(); + score(quality_score) + dimension_total + column_total + reference(a) + reference(b) + } + + let _ = delta; +} diff --git a/tests/report_delta.rs b/tests/report_delta.rs new file mode 100644 index 00000000..22bd6d9c --- /dev/null +++ b/tests/report_delta.rs @@ -0,0 +1,397 @@ +//! The typed report-to-report delta (#713). +//! +//! `python/tests/test_report_delta.py` is the Python twin, and also checks a +//! report loaded from a flat summary against the report it came from. + +use std::io::Write; + +use dataprof::{EvidenceGap, ProfileReport, Profiler, ReportDelta, ReportSide, StopCondition}; +use serde_json::{Value, json}; + +fn csv(contents: &str) -> tempfile::NamedTempFile { + let mut file = tempfile::Builder::new().suffix(".csv").tempfile().unwrap(); + file.write_all(contents.as_bytes()).unwrap(); + file +} + +fn profile(contents: &str) -> ProfileReport { + let file = csv(contents); + Profiler::new().analyze_file(file.path()).unwrap() +} + +fn document(delta: &ReportDelta) -> Value { + serde_json::to_value(delta).unwrap() +} + +const CLEAN: &str = "id,amount\n1,10\n2,20\n3,30\n4,40\n"; +const WITH_NULLS: &str = "id,amount\n1,10\n2,\n3,\n4,40\n"; + +#[test] +fn identical_reports_give_explicit_zeros_for_what_was_assessed() { + let report = profile(CLEAN); + let delta = document(&ReportDelta::between(&report, &report)); + + assert_eq!(delta["quality_score"]["abs"], json!(0.0)); + assert_eq!(delta["quality_score"]["rel_pct"], json!(0.0)); + for (name, change) in delta["dimensions"].as_object().unwrap() { + if change["a"].is_null() { + // Unassessed on both sides: no number to subtract. + assert!(change["abs"].is_null(), "{name}"); + } else { + assert_eq!(change["abs"], json!(0.0), "{name}"); + } + } + for change in delta["columns"].as_object().unwrap().values() { + assert_eq!(change["null_pct_delta"], json!(0.0)); + } + assert_eq!(delta["metric_semantics"]["comparable"], json!(true)); + assert_eq!(delta["schema"]["common"], json!(["id", "amount"])); +} + +#[test] +fn every_change_is_b_minus_a() { + let worse = profile(WITH_NULLS); + let better = profile(CLEAN); + let delta = document(&ReportDelta::between(&worse, &better)); + + let completeness = &delta["dimensions"]["completeness"]; + let (a, b) = ( + completeness["a"].as_f64().unwrap(), + completeness["b"].as_f64().unwrap(), + ); + assert!(b > a, "the clean side scores higher"); + assert!( + completeness["abs"].as_f64().unwrap() > 0.0, + "improvement is positive" + ); + assert_eq!(delta["columns"]["amount"]["null_pct_a"], json!(50.0)); + assert_eq!(delta["columns"]["amount"]["null_pct_b"], json!(0.0)); + assert_eq!( + delta["columns"]["amount"]["null_pct_delta"], + json!(-50.0), + "fewer nulls is negative" + ); +} + +#[test] +fn an_all_null_column_is_a_share_and_a_null_free_one_is_zero() { + let empty_column = profile("id,note\n1,\n2,\n"); + let filled = profile("id,note\n1,a\n2,b\n"); + let delta = document(&ReportDelta::between(&empty_column, &filled)); + + // The note column is all null: its share is 100%, a real number. + assert_eq!(delta["columns"]["note"]["null_pct_a"], json!(100.0)); + assert_eq!(delta["columns"]["note"]["null_pct_delta"], json!(-100.0)); + + let reversed = document(&ReportDelta::between(&filled, &empty_column)); + assert_eq!(reversed["columns"]["note"]["null_pct_a"], json!(0.0)); + assert_eq!(reversed["columns"]["note"]["null_pct_delta"], json!(100.0)); +} + +#[test] +fn a_report_with_nothing_assessed_invents_no_number() { + let header_only = profile("id,amount\n"); + let report = profile(CLEAN); + for delta in [ + document(&ReportDelta::between(&header_only, &report)), + document(&ReportDelta::between(&report, &header_only)), + document(&ReportDelta::between(&header_only, &header_only)), + ] { + assert!(delta["quality_score"]["abs"].is_null()); + assert!(delta["quality_score"]["rel_pct"].is_null()); + for change in delta["dimensions"].as_object().unwrap().values() { + assert!(change["abs"].is_null()); + assert!(change["rel_pct"].is_null()); + } + for change in delta["columns"].as_object().unwrap().values() { + assert!( + change["null_pct_delta"].is_null(), + "an empty column has no null share" + ); + } + } +} + +#[test] +fn schema_changes_keep_each_side_order() { + let a = profile("id,old,shared\n1,x,y\n"); + let b = profile("shared,new,id\ny,z,1\n"); + let delta = document(&ReportDelta::between(&a, &b)); + + assert_eq!(delta["schema"]["added"], json!(["new"])); + assert_eq!(delta["schema"]["removed"], json!(["old"])); + assert_eq!(delta["schema"]["common"], json!(["id", "shared"])); + let columns: Vec<&String> = delta["columns"].as_object().unwrap().keys().collect(); + assert_eq!(columns, ["id", "old", "shared", "new"]); + assert!(delta["columns"]["old"]["null_pct_b"].is_null()); + assert!(delta["columns"]["old"]["null_pct_delta"].is_null()); + assert!(delta["columns"]["new"]["null_pct_a"].is_null()); +} + +#[test] +fn a_truncated_side_says_so_on_every_number() { + let rows: String = (0..200).map(|i| format!("{i},{}\n", i * 2)).collect(); + let file = csv(&format!("id,amount\n{rows}")); + let exact = Profiler::new().analyze_file(file.path()).unwrap(); + let truncated = Profiler::new() + .stop_when(StopCondition::MaxRows(50)) + .analyze_file(file.path()) + .unwrap(); + + let delta = ReportDelta::between(&exact, &truncated); + assert!(delta.reports.a.evidence.is_complete()); + assert_eq!(delta.reports.b.evidence.gap(), Some(EvidenceGap::Truncated)); + assert_eq!(delta.reports.b.rows_processed, Some(50)); + assert!(delta.quality_score.evidence.a.is_complete()); + assert_eq!( + delta.quality_score.evidence.b.gap(), + Some(EvidenceGap::Truncated) + ); + for change in &delta.dimensions { + assert_eq!(change.score.evidence.b.gap(), Some(EvidenceGap::Truncated)); + } +} + +#[test] +fn a_summary_compares_like_the_report_it_came_from() { + let a = profile(WITH_NULLS); + let b = profile(CLEAN); + let native = document(&ReportDelta::between(&a, &b)); + let side = |report: &ProfileReport| { + ReportSide::from_summary_json(&report.summary_json().unwrap()).unwrap() + }; + let flat = document(&ReportDelta::between_sides(&side(&a), &side(&b))); + + let mut masked = native.clone(); + for name in ["a", "b"] { + // A flat summary records neither identity field. + masked["reports"][name]["id"] = Value::Null; + masked["reports"][name]["timestamp"] = Value::Null; + } + assert_eq!(flat, masked); + assert_eq!(native["reports"]["a"]["id"], json!(a.id)); +} + +#[test] +fn a_summary_from_before_0_12_reads_as_unrecorded() { + let legacy = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/legacy_0_11_non_ascii.summary.json"), + ) + .unwrap(); + let legacy = ReportSide::from_summary_json(&legacy).unwrap(); + let current = ReportSide::from_report(&profile("city\nOslo\n")); + let delta = ReportDelta::between_sides(&legacy, ¤t); + + assert_eq!(delta.metric_semantics.comparable, None); + assert!(delta.metric_semantics.a.is_none()); + // 0.11 did not record which quality components were sampled. + assert_eq!( + delta.quality_score.evidence.a.gap(), + Some(EvidenceGap::CoverageUnrecorded) + ); + assert!(delta.quality_score.evidence.b.is_complete()); +} + +#[test] +fn a_summary_without_columns_is_an_error() { + let report = profile(CLEAN); + let mut summary: Value = serde_json::from_str(&report.summary_json().unwrap()).unwrap(); + summary.as_object_mut().unwrap().remove("columns"); + let error = ReportSide::from_summary_json(&summary.to_string()).unwrap_err(); + assert!(error.to_string().contains("columns"), "{error}"); +} + +/// The oldest flat summaries record only some of the execution fields. Python +/// writes the rest as null; a summary from elsewhere can omit them. +#[test] +fn a_summary_that_did_not_record_its_scan_reads_as_unrecorded() { + let unrecorded = || Some(EvidenceGap::CoverageUnrecorded); + for execution in [ + json!({"rows_processed": 5}), + json!({"rows_processed": 5, "source_exhausted": null, "sampling_applied": null, "error_count": null}), + json!({}), + ] { + let summary = json!({"source": "old.csv", "columns": [], "execution": execution}); + let side = ReportSide::from_summary_json(&summary.to_string()).unwrap(); + let delta = ReportDelta::between_sides(&side, &side); + assert_eq!(delta.reports.a.evidence.gap(), unrecorded(), "{execution}"); + assert_eq!(delta.quality_score.evidence.a.gap(), unrecorded()); + assert!(delta.quality_score.abs.is_none()); + } + + let rows_only = json!({"source": "old.csv", "columns": [], "execution": {"rows_processed": 5}}); + let side = ReportSide::from_summary_json(&rows_only.to_string()).unwrap(); + assert_eq!( + ReportDelta::between_sides(&side, &side) + .reports + .a + .rows_processed, + Some(5) + ); + + // A recorded gap is still named over an unrecorded fact. + let truncated = json!({"source": "x", "columns": [], "execution": {"source_exhausted": false}}); + let side = ReportSide::from_summary_json(&truncated.to_string()).unwrap(); + assert_eq!( + ReportDelta::between_sides(&side, &side) + .reports + .a + .evidence + .gap(), + Some(EvidenceGap::Truncated) + ); +} + +/// Without the weights behind the overall score, a sampled component may +/// reach it, so the score is not certified exact. +#[test] +fn an_overall_score_without_its_weights_is_not_certified() { + let report = profile(CLEAN); + let mut summary: Value = serde_json::from_str(&report.summary_json().unwrap()).unwrap(); + summary["quality"]["sampled_dimensions"] = json!(["consistency"]); + for assessed in [None, Some(json!([]))] { + let quality = summary["quality"].as_object_mut().unwrap(); + match &assessed { + None => { + quality.remove("assessed_dimensions"); + } + Some(list) => { + quality.insert("assessed_dimensions".into(), list.clone()); + } + } + let side = ReportSide::from_summary_json(&summary.to_string()).unwrap(); + let delta = ReportDelta::between_sides(&side, &side); + assert_eq!( + delta.quality_score.evidence.a.gap(), + Some(EvidenceGap::CoverageUnrecorded), + "assessed_dimensions = {assessed:?}" + ); + } + + // Nothing sampled settles it without the weights. + summary["quality"]["sampled_dimensions"] = json!([]); + let side = ReportSide::from_summary_json(&summary.to_string()).unwrap(); + assert!( + ReportDelta::between_sides(&side, &side) + .quality_score + .evidence + .a + .is_complete() + ); +} + +/// A rebuilt report can repeat a column name. The delta lists it once and +/// reads its first profile, as `QualityPolicy` does. +#[test] +fn a_repeated_column_name_is_listed_once_with_its_first_profile() { + let mut report = profile(CLEAN); + let mut first = report.column_profiles[1].clone(); + let mut second = first.clone(); + first.null_count = 0; + second.null_count = second.total_count; + report.column_profiles = vec![report.column_profiles[0].clone(), first, second]; + + let delta = document(&ReportDelta::between(&report, &report)); + assert_eq!(delta["schema"]["common"], json!(["id", "amount"])); + let columns: Vec<&String> = delta["columns"].as_object().unwrap().keys().collect(); + assert_eq!(columns, ["id", "amount"]); + assert_eq!(delta["columns"]["amount"]["null_pct_a"], json!(0.0)); + + let side = ReportSide::from_summary_json(&report.summary_json().unwrap()).unwrap(); + let flat = document(&ReportDelta::between_sides(&side, &side)); + assert_eq!(flat["schema"], delta["schema"]); + assert_eq!(flat["columns"], delta["columns"]); +} + +#[test] +fn serialization_is_deterministic_and_ordered() { + let a = profile(WITH_NULLS); + let b = profile(CLEAN); + let first = serde_json::to_string(&ReportDelta::between(&a, &b)).unwrap(); + let second = serde_json::to_string(&ReportDelta::between(&a, &b)).unwrap(); + assert_eq!(first, second); + + let delta: Value = serde_json::from_str(&first).unwrap(); + let keys: Vec<&String> = delta.as_object().unwrap().keys().collect(); + assert_eq!( + keys, + [ + "quality_score", + "dimensions", + "columns", + "metric_semantics", + "schema", + "reports" + ] + ); + let dimensions: Vec<&String> = delta["dimensions"].as_object().unwrap().keys().collect(); + assert_eq!( + dimensions, + [ + "completeness", + "consistency", + "uniqueness", + "accuracy", + "timeliness", + "validity", + "precision" + ] + ); +} + +#[test] +fn a_partly_recorded_side_leaves_comparability_unknown() { + let report = profile(CLEAN); + let mut summary: Value = serde_json::from_str(&report.summary_json().unwrap()).unwrap(); + // 0.12 recorded only its text-length unit. + summary["metric_semantics"] = json!({"text_length_unit": "unicode_scalar"}); + let partial = ReportSide::from_summary_json(&summary.to_string()).unwrap(); + let current = ReportSide::from_report(&report); + + for delta in [ + ReportDelta::between_sides(&partial, ¤t), + ReportDelta::between_sides(¤t, &partial), + ReportDelta::between_sides(&partial, &partial), + ] { + assert_eq!(delta.metric_semantics.comparable, None); + } + let both = ReportDelta::between_sides(¤t, ¤t); + assert_eq!(both.metric_semantics.comparable, Some(true)); +} + +/// A dimension a later release added carries weight in the stored overall +/// score. When its component was sampled, nothing this build knows rules out +/// that the sample reaches the score, so the score is not certified exact. +#[test] +fn a_sampled_component_this_build_does_not_know_leaves_the_overall_unrecorded() { + let report = profile(CLEAN); + let mut canonical = serde_json::to_value(&report).unwrap(); + canonical["quality"]["confidence"] = json!({"Mixed": { + "exact_dimensions": ["completeness"], + "sample_size": 4, + "sampled_dimensions": ["future_dimension"], + }}); + let native: ProfileReport = serde_json::from_value(canonical).unwrap(); + let delta = ReportDelta::between(&native, &native); + assert_eq!( + delta.quality_score.evidence.a.gap(), + Some(EvidenceGap::CoverageUnrecorded) + ); + // A known dimension's own score is unaffected. + let completeness = &delta.dimensions[0]; + assert!(completeness.score.evidence.a.is_complete()); + + let mut summary: Value = serde_json::from_str(&report.summary_json().unwrap()).unwrap(); + summary["quality"]["sampled_dimensions"] = json!(["future_dimension"]); + summary["quality"]["assessed_dimensions"] = json!(["completeness", "future_dimension"]); + let side = ReportSide::from_summary_json(&summary.to_string()).unwrap(); + assert_eq!( + ReportDelta::between_sides(&side, &side) + .quality_score + .evidence + .a + .gap(), + Some(EvidenceGap::CoverageUnrecorded) + ); +} diff --git a/tests/timeliness_declared_pairs.rs b/tests/timeliness_declared_pairs.rs new file mode 100644 index 00000000..843e899a --- /dev/null +++ b/tests/timeliness_declared_pairs.rs @@ -0,0 +1,243 @@ +//! Start/end pairs declared next to `temporal_columns` (#942). +//! +//! Timeliness found pairs only from role words in the names, so +//! `valid_from`/`valid_until`, `checkin`/`checkout` or names in another +//! language were never compared. A declared pair is compared as given on +//! every engine and input format, and an unknown name in one is reported like +//! any other hint. +//! +//! `python/tests/test_timeliness_declared_pairs.py` is the Python twin. + +use std::io::Write; + +use dataprof::{DataProfilerError, EngineType, ProfileReport, Profiler, SemanticHintKind}; +use tempfile::NamedTempFile; + +const START: &str = "valid_from"; +const END: &str = "valid_until"; +/// The start of every row. +const STARTS: [&str; 3] = ["2024-03-15", "2024-04-20", "2024-05-10"]; +/// The end, before the start on every row, so the pair counts three violations. +const ENDS: [&str; 3] = ["2024-02-01", "2024-03-01", "2024-03-20"]; + +fn text_fixture(suffix: &str, body: String) -> NamedTempFile { + let mut file = NamedTempFile::with_suffix(suffix).unwrap(); + write!(file, "{body}").unwrap(); + file.flush().unwrap(); + file +} + +fn csv_fixture() -> NamedTempFile { + let rows = STARTS + .iter() + .zip(ENDS) + .map(|(start, end)| format!("{start},{end}")); + let body = std::iter::once(format!("{START},{END}")) + .chain(rows) + .collect::>() + .join("\n") + + "\n"; + text_fixture(".csv", body) +} + +#[cfg(feature = "parquet")] +fn parquet_fixture() -> NamedTempFile { + use std::sync::Arc; + + use arrow::array::{ArrayRef, StringArray}; + use arrow::compute::cast; + use arrow::datatypes::{DataType, Field, Schema}; + use arrow::record_batch::RecordBatch; + use parquet::arrow::ArrowWriter; + + let days = |values: [&str; 3]| -> ArrayRef { + cast(&StringArray::from(values.to_vec()), &DataType::Date32).unwrap() + }; + let schema = Arc::new(Schema::new(vec![ + Field::new(START, DataType::Date32, false), + Field::new(END, DataType::Date32, false), + ])); + let batch = RecordBatch::try_new(schema.clone(), vec![days(STARTS), days(ENDS)]).unwrap(); + + let file = NamedTempFile::with_suffix(".parquet").unwrap(); + let mut writer = ArrowWriter::try_new(file.reopen().unwrap(), schema, None).unwrap(); + writer.write(&batch).unwrap(); + writer.close().unwrap(); + file +} + +fn declared(start: &str, end: &str) -> Vec<(String, String)> { + vec![(start.to_string(), end.to_string())] +} + +/// Every engine over the CSV, then every other format, each profiled with +/// `pairs` declared. +fn reports(pairs: &[(String, String)]) -> Vec<(String, ProfileReport)> { + let csv = csv_fixture(); + let mut reports = Vec::new(); + for engine in [ + EngineType::Auto, + EngineType::Incremental, + EngineType::Columnar, + ] { + let report = Profiler::new() + .engine(engine) + .temporal_pairs(pairs.to_vec()) + .analyze_file(csv.path()) + .unwrap_or_else(|e| panic!("[{engine:?}] {e}")); + reports.push((format!("{engine:?}"), report)); + } + + let records: Vec = STARTS + .iter() + .zip(ENDS) + .map(|(start, end)| format!(r#"{{"{START}":"{start}","{END}":"{end}"}}"#)) + .collect(); + let mut files = vec![ + ( + "json", + text_fixture(".json", format!("[{}]", records.join(","))), + ), + ("jsonl", text_fixture(".jsonl", records.join("\n") + "\n")), + ]; + #[cfg(feature = "parquet")] + files.push(("parquet", parquet_fixture())); + for (label, file) in files { + let report = Profiler::new() + .temporal_pairs(pairs.to_vec()) + .analyze_file(file.path()) + .unwrap_or_else(|e| panic!("[{label}] {e}")); + reports.push((label.to_string(), report)); + } + reports +} + +/// `(temporal_pairs_checked, temporal_violations)`. +fn ordering(report: &ProfileReport, label: &str) -> (usize, usize) { + let timeliness = report + .quality + .as_ref() + .and_then(|quality| quality.metrics.timeliness.as_ref()) + .unwrap_or_else(|| panic!("[{label}] timeliness assessed")); + ( + timeliness.temporal_pairs_checked, + timeliness.temporal_violations, + ) +} + +/// Collects every wrong path before failing, so a regression names all the +/// paths it reaches rather than the first. +#[test] +fn a_declared_pair_is_compared_on_every_path() { + let mut wrong = Vec::new(); + for (pairs, expected) in [(Vec::new(), (0, 0)), (declared(START, END), (3, 3))] { + for (label, report) in reports(&pairs) { + let found = ordering(&report, &label); + if found != expected { + wrong.push(format!( + "[{label}] declared {pairs:?}: (pairs_checked, violations) {found:?}, expected {expected:?}" + )); + } + // Each pair column binds once, as the option the caller used. + let bound: Vec<(&str, usize)> = report + .semantic_hint_bindings + .iter() + .filter(|binding| binding.kind == SemanticHintKind::TemporalPair) + .map(|binding| (binding.column.as_str(), binding.matched_values)) + .collect(); + let expected_bound = if pairs.is_empty() { + vec![] + } else { + vec![(START, 3), (END, 3)] + }; + if bound != expected_bound { + wrong.push(format!( + "[{label}] declared {pairs:?}: bindings {bound:?}, expected {expected_bound:?}" + )); + } + } + } + assert!(wrong.is_empty(), "{}", wrong.join("\n")); +} + +#[track_caller] +fn assert_hint_error(result: Result, needles: &[&str]) { + match result { + Err(DataProfilerError::InvalidSemanticHint { + message, + suggestion, + }) => { + let text = format!("{message}\n{suggestion}"); + for needle in needles { + assert!(text.contains(needle), "expected {needle:?} in: {text}"); + } + } + Err(other) => panic!("expected InvalidSemanticHint, got: {other}"), + Ok(_) => panic!("expected InvalidSemanticHint, but profiling succeeded"), + } +} + +#[test] +fn an_unknown_column_in_a_declared_pair_is_rejected() { + let csv = csv_fixture(); + for engine in [EngineType::Incremental, EngineType::Columnar] { + let result = Profiler::new() + .engine(engine) + .temporal_pairs(declared(START, "valid_untill")) + .analyze_file(csv.path()); + assert_hint_error( + result, + &["'valid_untill'", "temporal_pairs", "'valid_until'"], + ); + } +} + +#[test] +fn a_pair_of_one_column_with_itself_is_rejected() { + let csv = csv_fixture(); + let result = Profiler::new() + .temporal_pairs(declared(START, START)) + .analyze_file(csv.path()); + assert_hint_error(result, &["'valid_from'", "temporal_pairs"]); +} + +#[test] +fn a_declared_pair_over_columns_without_dates_is_rejected() { + // The clean check that never happened: two text columns used to profile + // fine and report `temporal_pairs_checked == 0`. + let body = "name,city\nAda,Turin\nLinus,Helsinki\nGrace,Arlington\n"; + let csv = text_fixture(".csv", body.to_string()); + let records = r#"{"name":"Ada","city":"Turin"} +{"name":"Linus","city":"Helsinki"}"#; + let jsonl = text_fixture(".jsonl", records.to_string() + "\n"); + let mut runs = vec![ + ("Auto", Profiler::new().engine(EngineType::Auto), csv.path()), + ( + "Incremental", + Profiler::new().engine(EngineType::Incremental), + csv.path(), + ), + ( + "Columnar", + Profiler::new().engine(EngineType::Columnar), + csv.path(), + ), + ]; + runs.push(("jsonl", Profiler::new(), jsonl.path())); + for (label, profiler, path) in runs { + let result = profiler + .temporal_pairs(declared("name", "city")) + .analyze_file(path); + match result { + Err(DataProfilerError::InvalidSemanticHint { message, .. }) => { + for needle in ["'name' (temporal_pairs)", "'city' (temporal_pairs)"] { + assert!( + message.contains(needle), + "[{label}] {needle:?} in: {message}" + ); + } + } + other => panic!("[{label}] expected InvalidSemanticHint, got {other:?}"), + } + } +}