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: adjust Signature::name() to return Option<String> instead of filename() and md5sum() #3434

Merged
merged 19 commits into from
Dec 18, 2024
Merged
9 changes: 8 additions & 1 deletion src/core/src/index/revindex/disk_revindex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,14 @@
.collection
.record_for_dataset(dataset_id)
.expect("dataset not found");
Some((row.name().into(), size))

let name = [row.name(), row.filename(), row.md5()]
.into_iter()
.skip_while(|v| v.is_empty())

Check warning on line 357 in src/core/src/index/revindex/disk_revindex.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/index/revindex/disk_revindex.rs#L357

Added line #L357 was not covered by tests
.next()
.unwrap(); // guaranteed to succeed because `md5` always exists

Check warning on line 359 in src/core/src/index/revindex/disk_revindex.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/index/revindex/disk_revindex.rs#L359

Added line #L359 was not covered by tests

Some((name.into(), size))
} else {
None
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/index/revindex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ mod test {
)?;

assert_eq!(matches.len(), 1);
assert_eq!(matches[0].name(), "../genome-s10.fa.gz");
assert_eq!(matches[0].name(), ""); // signature name is empty
assert_eq!(matches[0].f_match(), 1.0);

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl Record {
Self {
internal_location: path.into(),
moltype: moltype.to_string(),
name: sig.name(),
name: sig.name_str(),
ksize,
md5,
md5short,
Expand Down
17 changes: 9 additions & 8 deletions src/core/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,14 +445,13 @@
}

impl Signature {
pub fn name(&self) -> String {
if let Some(name) = &self.name {
name.clone()
} else if let Some(filename) = &self.filename {
filename.clone()
} else {
self.md5sum()
}
pub fn name(&self) -> Option<String> {
self.name.clone()
}

/// return name, if not None; or "" if None.
pub fn name_str(&self) -> String {
self.name().unwrap_or("".into())

Check warning on line 454 in src/core/src/signature.rs

View check run for this annotation

Codecov / codecov/patch

src/core/src/signature.rs#L454

Added line #L454 was not covered by tests
}

pub fn set_name(&mut self, name: &str) {
Expand Down Expand Up @@ -982,6 +981,8 @@
assert_eq!(sig.signatures[0].size(), 3);
assert_eq!(sig.signatures[1].size(), 2);
assert_eq!(sig.signatures[2].size(), 1);

assert_eq!(sig.name_str(), "");
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/core/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ impl ZipStorage {

impl SigStore {
pub fn new_with_storage(sig: Signature, storage: InnerStorage) -> Self {
let name = sig.name();
let name = sig.name_str();
let filename = sig.filename();

SigStore::builder()
Expand Down Expand Up @@ -555,7 +555,7 @@ impl Deref for SigStore {

impl From<Signature> for SigStore {
fn from(other: Signature) -> SigStore {
let name = other.name();
let name = other.name_str();
let filename = other.filename();

SigStore::builder()
Expand Down
2 changes: 1 addition & 1 deletion src/core/tests/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fn innerstorage_save_sig() -> Result<(), Box<dyn std::error::Error>> {

let loaded_sig = instorage.load_sig("test")?;

assert_eq!(sig.name(), loaded_sig.name());
assert_eq!(sig.name_str(), loaded_sig.name());
assert_eq!(sig.md5sum(), loaded_sig.md5sum());

Ok(())
Expand Down
Loading