chore: code tidy and script tidy

.github/workflows/ci.yml

@@ -93,7 +93,7 @@ jobs:
           cache-bin: "false"
       - name: Prepare test data
         shell: bash
-        run: ./tools/generate_serialization_test_data.py
+        run: ./tools/download_serialization_test_data.py
       - name: Run unit tests
         shell: bash
         run: cargo x test

CONTRIBUTING.md

@@ -88,7 +88,7 @@ cargo install taplo-cli typos-cli hawkeye
 Some tests depend on snapshot files under `datasketches/tests/serialization_test_data`. If they are missing, tests will fail. Download them with:
 
 ```shell
-python3 ./tools/generate_serialization_test_data.py --all
+python3 ./tools/download_serialization_test_data.py --all
 ```
 
 The script downloads the latest snapshots from the `main` branch of
@@ -100,8 +100,8 @@ The script downloads the latest snapshots from the `main` branch of
 You can download them separately:
 
 ```shell
-python3 ./tools/generate_serialization_test_data.py --java
-python3 ./tools/generate_serialization_test_data.py --cpp
+python3 ./tools/download_serialization_test_data.py --java
+python3 ./tools/download_serialization_test_data.py --cpp
 ```
 
 The script requires Python 3 and network access.

datasketches/src/tuple/hash_table.rs

@@ -20,6 +20,8 @@ use std::num::NonZeroU64;
 
 use crate::thetacommon::RawHashTableEntry;
 use crate::thetacommon::hash_table::RawHashTable;
+use crate::thetacommon::union::RawThetaUnionPolicy;
+use crate::tuple::SummaryCombinePolicy;
 
 /// A retained entry in a Tuple sketch: a hash key together with its associated summary.
 #[derive(Debug, Clone)]
@@ -37,7 +39,7 @@ impl<S> TupleEntry<S> {
     /// # Panics
     ///
     /// Panics if `hash` is zero.
-    pub(crate) fn new(hash: u64, summary: S) -> Self {
+    pub(super) fn new(hash: u64, summary: S) -> Self {
         let hash = NonZeroU64::new(hash).expect("hash must be non-zero");
         Self { hash, summary }
     }
@@ -51,11 +53,6 @@ impl<S> TupleEntry<S> {
     pub fn summary(&self) -> &S {
         &self.summary
     }
-
-    /// Returns a mutable reference to the summary stored in this entry.
-    pub(super) fn summary_mut(&mut self) -> &mut S {
-        &mut self.summary
-    }
 }
 
 /// Specific hash table for tuple sketch.
@@ -110,6 +107,12 @@ impl<S> TupleHashTable<S> {
     }
 }
 
+impl<P: SummaryCombinePolicy> RawThetaUnionPolicy<TupleEntry<P::Summary>> for P {
+    fn merge(&self, existing: &mut TupleEntry<P::Summary>, incoming: TupleEntry<P::Summary>) {
+        self.combine(&mut existing.summary, &incoming.summary);
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

datasketches/src/tuple/union.rs

@@ -29,25 +29,11 @@ use crate::thetacommon::constants::DEFAULT_LG_K;
 use crate::thetacommon::constants::MAX_LG_K;
 use crate::thetacommon::constants::MIN_LG_K;
 use crate::thetacommon::union::RawThetaUnion;
-use crate::thetacommon::union::RawThetaUnionPolicy;
 use crate::tuple::hash_table::TupleEntry;
 use crate::tuple::policy::SummaryCombinePolicy;
 use crate::tuple::sketch::CompactTupleSketch;
 use crate::tuple::sketch::TupleSketchView;
 
-/// Adapts a [`SummaryCombinePolicy`] to the raw union's entry-merge policy.
-#[derive(Debug)]
-struct CombinePolicyAdapter<P>(P);
-
-impl<P> RawThetaUnionPolicy<TupleEntry<P::Summary>> for CombinePolicyAdapter<P>
-where
-    P: SummaryCombinePolicy,
-{
-    fn merge(&self, existing: &mut TupleEntry<P::Summary>, incoming: TupleEntry<P::Summary>) {
-        self.0.combine(existing.summary_mut(), incoming.summary());
-    }
-}
-
 /// Union (set OR) of Tuple sketches.
 ///
 /// `P` is the [`SummaryCombinePolicy`] applied when a key is present in more than one input. For
@@ -79,7 +65,7 @@ pub struct TupleUnion<P>
 where
     P: SummaryCombinePolicy,
 {
-    raw: RawThetaUnion<TupleEntry<P::Summary>, CombinePolicyAdapter<P>>,
+    raw: RawThetaUnion<TupleEntry<P::Summary>, P>,
 }
 
 impl<P> TupleUnion<P>
@@ -217,7 +203,7 @@ where
                 self.resize_factor,
                 self.sampling_probability,
                 self.seed,
-                CombinePolicyAdapter(self.policy),
+                self.policy,
             ),
         }
     }

datasketches/tests/common.rs

@@ -39,10 +39,10 @@ pub fn serialization_test_data(sub_dir: &str, name: &str) -> PathBuf {
             r#"serialization test data file not found: {}
 
             Please ensure test data files are present in the repository. Generally, you can
-            run the following commands from the project root to regenerate the test data files
+            run the following commands from the project root to download the test data files
             if they are missing:
 
-            $ ./tools/generate_serialization_test_data.py
+            $ ./tools/download_serialization_test_data.py
         "#,
             path.display(),
         );

datasketches/tests/tuple_serialization_test.rs

@@ -15,21 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-//! Cross-language compatibility tests for Tuple sketch serialization.
-//!
-//! The fixtures are produced by the upstream Java and C++ generators (see
-//! `tools/generate_serialization_test_data.py`):
-//!
-//! * Java: `TupleCrossLanguageTest.generateForCppIntegerSummary` writes `tuple_int_n{n}_java.sk`
-//!   using its `IntegerSummary`.
-//! * C++: `tuple_sketch_serialize_for_java.cpp` writes `tuple_int_n{n}_cpp.sk` using an `int`
-//!   summary.
-//!
-//! Both build a tuple sketch with `update(i, i)` for `i` in `0..n`, so the summary is a 4-byte
-//! little-endian signed integer — exactly what the `i32` [`TupleSummaryValue`] implementation
-//! reads. The `aod_*`/`aos_*` fixtures use Array-of-Doubles / Array-of-Strings summaries, which
-//! this crate does not implement, so they are intentionally not covered here.
-
 #![cfg(feature = "tuple")]
 
 mod common;

datasketches/tests/tuple_sketch_test.rs

@@ -15,11 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-//! Behavioral tests for the Tuple sketch, mirroring `theta_sketch_test.rs`.
-//!
-//! Updates carry a `u64` summary combined with the default (additive) policy, so the distinct-count
-//! behavior matches the Theta sketch while the summaries accumulate alongside each key.
-
 #![cfg(feature = "tuple")]
 
 use datasketches::common::NumStdDev;

tools/generate_serialization_test_data.py → tools/download_serialization_test_data.py

@@ -26,9 +26,7 @@
 import zipfile
 from pathlib import Path, PurePosixPath
 
-TCK_ARCHIVE_URL = (
-    "https://github.com/apache/datasketches-tck/archive/refs/heads/main.zip"
-)
+TCK_ARCHIVE_URL = "https://github.com/apache/datasketches-tck/archive/0016a517/main.zip"
 OUTPUT_DIRS = {
     "java": "java_generated_files",
     "cpp": "cpp_generated_files",
@@ -37,16 +35,13 @@
 
 def download_archive(destination):
     print(f"Downloading serialization snapshots from {TCK_ARCHIVE_URL}", flush=True)
-    request = urllib.request.Request(
-        TCK_ARCHIVE_URL,
-        headers={"User-Agent": "apache-datasketches-rust"},
-    )
+    request = urllib.request.Request(TCK_ARCHIVE_URL)
     with urllib.request.urlopen(request, timeout=60) as response:
         with destination.open("wb") as output:
             shutil.copyfileobj(response, output)
 
 
-def install_snapshots(archive, project_dir, language):
+def extract_snapshots(archive, project_dir, language):
     source_parts = ("serialization", language, "snapshots")
     members = []
 
@@ -95,7 +90,7 @@ def install_snapshots(archive, project_dir, language):
             if temp_path is not None and temp_path.exists():
                 temp_path.unlink()
 
-    print(f"Installed {len(members)} {language} snapshots into {output_dir}")
+    print(f"Extracted {len(members)} {language} snapshots into {output_dir}")
 
 
 def main():
@@ -115,18 +110,15 @@ def main():
     if not languages:
         languages = list(OUTPUT_DIRS)
 
-    project_dir = Path(__file__).resolve().parent.parent / "datasketches"
-
-    try:
-        with tempfile.TemporaryDirectory(prefix="datasketches-tck-") as temp_dir:
-            archive_path = Path(temp_dir) / "datasketches-tck.zip"
-            download_archive(archive_path)
-            with zipfile.ZipFile(archive_path) as archive:
-                for language in languages:
-                    install_snapshots(archive, project_dir, language)
-    except (OSError, RuntimeError, urllib.error.URLError, zipfile.BadZipFile) as error:
-        print(f"Error: {error}", file=sys.stderr)
-        return 1
+    repository_root = Path(__file__).resolve().parents[1]
+    project_dir = repository_root / "datasketches"
+
+    with tempfile.TemporaryDirectory(prefix="datasketches-tck-") as temp_dir:
+        archive_path = Path(temp_dir) / "datasketches-tck.zip"
+        download_archive(archive_path)
+        with zipfile.ZipFile(archive_path) as archive:
+            for language in languages:
+                extract_snapshots(archive, project_dir, language)
 
     return 0
 

typos.toml

@@ -18,6 +18,7 @@
 [default.extend-words]
 # False-Positive Abbreviations
 "PREINTS" = "PREINTS"
+"SER" = "SER"
 
 [files]
 extend-exclude = []