Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MRG: add Manifest::intersect_manifest to Rust core #3305

Merged
merged 40 commits into from
Sep 21, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
6b1d1bb
make error catchable
ctb Aug 21, 2024
7e73e8f
flag another problematic unwrap
ctb Aug 21, 2024
2896c49
more
ctb Aug 21, 2024
45b1a8f
more
ctb Aug 21, 2024
e8d7999
upd log message
ctb Aug 21, 2024
53bcf02
provide correct error
ctb Aug 21, 2024
d467d9f
switch Manifest from paths to TryFrom
ctb Aug 22, 2024
5f1eef6
Revert "provide correct error"
ctb Aug 22, 2024
eb46ecd
Revert "switch Manifest from paths to TryFrom"
ctb Aug 22, 2024
923af44
poor person's picklist?
ctb Aug 23, 2024
315dfff
add picklist select
ctb Aug 23, 2024
1fb658a
ok
ctb Aug 23, 2024
a122982
picklist by ref
ctb Aug 23, 2024
39816ab
add manifest.is_empty()
ctb Aug 23, 2024
61416fb
update revindex indexing message
ctb Aug 24, 2024
3a7abe9
propagate error on bad directory when opening RocksDB
ctb Aug 24, 2024
07e3a09
Merge branch 'update_revindex' into remove_unwrap
ctb Aug 24, 2024
fe75c6c
do we not need len?
ctb Aug 24, 2024
5fb20fc
nope, don't need em
ctb Aug 24, 2024
2c59010
revert for now
ctb Aug 24, 2024
c8477df
Merge branch 'latest' of github.com:sourmash-bio/sourmash into remove…
ctb Aug 27, 2024
6fee403
adjust select_picklist per luiz
ctb Aug 27, 2024
0675402
simplify and encapsulate
ctb Aug 27, 2024
17f50ef
cargo fmt
ctb Aug 27, 2024
39c140b
add a test of intersect_manifest
ctb Sep 15, 2024
16f72c5
impl PartialEq/Eq for Record, ignoring internal_location
ctb Sep 15, 2024
2146f30
switch to using full Record for intersect_manifest
ctb Sep 16, 2024
32839ae
round out comparison & hashing for Record
ctb Sep 16, 2024
43ee757
cargo fmt
ctb Sep 16, 2024
31aa378
Merge branch 'latest' into remove_unwrap
ctb Sep 16, 2024
1594dc4
remove identity closure
ctb Sep 16, 2024
2465c02
add in a print for debugging
ctb Sep 16, 2024
5af5f45
more print
ctb Sep 16, 2024
b46565d
remove prints
ctb Sep 16, 2024
b7a9850
Merge branch 'latest' of github.com:sourmash-bio/sourmash into remove…
ctb Sep 17, 2024
9c26752
add a test for Collection::intersect_manifest
ctb Sep 17, 2024
f2fde5d
cargo fmt
ctb Sep 17, 2024
f4e89f4
modify intersect_manifest signature
ctb Sep 18, 2024
9b9e17e
mut Collection for intersect_manifest
ctb Sep 18, 2024
aa109f8
omit unit return type
ctb Sep 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/core/src/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use camino::Utf8Path as Path;
use camino::Utf8PathBuf as PathBuf;
use std::collections::HashSet;

use crate::encodings::Idx;
use crate::manifest::{Manifest, Record};
Expand Down Expand Up @@ -215,6 +216,12 @@
assert_eq!(sig.signatures.len(), 1);
Ok(sig)
}

pub fn select_picklist(&self, pick: &HashSet<(String, String)>) -> Self {

Check warning on line 220 in src/core/src/collection.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/collection.rs#L220

Added line #L220 was not covered by tests
// @CTB: why do we need this clone here?
ctb marked this conversation as resolved.
Show resolved Hide resolved
let manifest = self.manifest.clone().select_picklist(pick);
Self { manifest, storage: self.storage.clone() }

Check warning on line 223 in src/core/src/collection.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/collection.rs#L222-L223

Added lines #L222 - L223 were not covered by tests
}
}

impl Select for Collection {
Expand All @@ -236,7 +243,7 @@
use crate::prelude::Select;
use crate::selection::Selection;
use crate::signature::Signature;
use crate::Result;

Check warning on line 246 in src/core/src/collection.rs

View workflow job for this annotation

GitHub Actions / test (stable)

unused import: `crate::Result`

Check warning on line 246 in src/core/src/collection.rs

View workflow job for this annotation

GitHub Actions / test (beta)

unused import: `crate::Result`

Check warning on line 246 in src/core/src/collection.rs

View workflow job for this annotation

GitHub Actions / test (windows)

unused import: `crate::Result`

Check warning on line 246 in src/core/src/collection.rs

View workflow job for this annotation

GitHub Actions / test (macos)

unused import: `crate::Result`

#[test]
fn sigstore_selection_with_downsample() {
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/index/revindex/disk_revindex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl RevIndex {

info!("Compact SSTs");
index.compact();
info!("Processed {} reference sigs", processed_sigs.into_inner());
info!("Done! Processed {} reference sigs", processed_sigs.into_inner());

Ok(module::RevIndex::Plain(index))
}
Expand Down
12 changes: 11 additions & 1 deletion src/core/src/index/revindex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl RevIndex {

pub fn open<P: AsRef<Path>>(index: P, read_only: bool, spec: Option<&str>) -> Result<Self> {
let opts = db_options();
let cfs = DB::list_cf(&opts, index.as_ref()).unwrap();
let cfs = DB::list_cf(&opts, index.as_ref())?;

if cfs.into_iter().any(|c| c == COLORS) {
// TODO: ColorRevIndex can't be read-only for now,
Expand Down Expand Up @@ -1020,4 +1020,14 @@ mod test {

Ok(())
}

#[test]
fn rocksdb_storage_fail_bad_directory() -> Result<()> {
let testdir = TempDir::new()?;

match RevIndex::open(testdir, true, None) {
Err(_) => Ok(()),
Ok(_) => panic!("test should not reach here"),
}
}
}
9 changes: 9 additions & 0 deletions src/core/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use rayon::prelude::*;
use serde::de;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;

use crate::encodings::HashFunctions;
use crate::prelude::*;
Expand Down Expand Up @@ -209,6 +210,14 @@
pub fn iter(&self) -> impl Iterator<Item = &Record> {
self.records.iter()
}

pub fn select_picklist(self, pick: &HashSet<(String, String)>) -> Self {
let records = self.records.iter().filter(|row| {
pick.contains(&(row.name().clone(), row.md5().clone()))
}).cloned().collect();

Check warning on line 217 in src/core/src/manifest.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L214-L217

Added lines #L214 - L217 were not covered by tests

Self { records }
}
}

impl Select for Manifest {
Expand Down
Loading