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: make core Manifest booleans python compatible (core) #3007

Merged
merged 6 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 56 additions & 7 deletions src/core/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

md5short: String,

#[getset(get = "pub", set = "pub")]
#[getset(get_copy = "pub", set = "pub")]
Copy link
Member

Choose a reason for hiding this comment

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

Change this to get_copy, for basic types like u32 and bool it makes the API easier (they are trivial to copy)

ksize: u32,

moltype: String,
Expand All @@ -35,8 +35,8 @@
scaled: u64,
n_hashes: usize,

#[getset(get = "pub", set = "pub")]
#[serde(deserialize_with = "to_bool")]
#[getset(get_copy = "pub", set = "pub")]
#[serde(serialize_with = "intbool", deserialize_with = "to_bool")]
with_abundance: bool,

#[getset(get = "pub", set = "pub")]
Expand All @@ -45,6 +45,17 @@
filename: String,
}

fn intbool<S>(x: &bool, s: S) -> std::result::Result<S::Ok, S::Error>

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

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L48

Added line #L48 was not covered by tests
where
S: serde::Serializer,
{
if *x {
s.serialize_i32(1)

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

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L53

Added line #L53 was not covered by tests
} else {
s.serialize_i32(0)

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

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L55

Added line #L55 was not covered by tests
}
}

fn to_bool<'de, D>(deserializer: D) -> std::result::Result<bool, D::Error>
where
D: de::Deserializer<'de>,
Expand All @@ -53,11 +64,11 @@
.to_ascii_lowercase()
.as_ref()
{
"0" | "false" => Ok(false),
"1" | "true" => Ok(true),
"0" | "false" | "False" => Ok(false),
"1" | "true" | "True" => Ok(true),
other => Err(de::Error::invalid_value(
de::Unexpected::Str(other),
&"0/1 or true/false are the only supported values",
&"0/1, true/false, True/False are the only supported values",

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

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L71

Added line #L71 was not covered by tests
)),
}
}
Expand Down Expand Up @@ -192,7 +203,7 @@
valid
};
valid = if let Some(abund) = selection.abund() {
valid && *row.with_abundance() == abund
valid && row.with_abundance() == abund

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

View check run for this annotation

Codecov / codecov/patch

src/core/src/manifest.rs#L206

Added line #L206 was not covered by tests
} else {
valid
};
Expand Down Expand Up @@ -387,4 +398,42 @@
// load into manifest
let _manifest = Manifest::from(&full_paths[..]); // pass full_paths as a slice
}

#[test]
fn test_manifest_to_writer_bools() {
let base_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));

let test_sigs = vec![
PathBuf::from("../../tests/test-data/47.fa.sig"),
PathBuf::from("../../tests/test-data/track_abund/63.fa.sig"),
];

let full_paths: Vec<PathBuf> = test_sigs
.into_iter()
.map(|sig| base_path.join(sig))
.collect();

let manifest = Manifest::from(&full_paths[..]); // pass full_paths as a slice

let temp_dir = TempDir::new().unwrap();
let utf8_output = PathBuf::from_path_buf(temp_dir.path().to_path_buf())
.expect("Path should be valid UTF-8");

let filename = utf8_output.join("sigs.manifest.csv");
let mut wtr = File::create(&filename).expect("Failed to create file");

manifest.to_writer(&mut wtr).unwrap();

// check that we can reopen the file as a manifest + properly check abund
let infile = File::open(&filename).expect("Failed to open file");
let m2 = Manifest::from_reader(&infile).unwrap();
for record in m2.iter() {
eprintln!("{:?}", record.name());
if record.name().contains("OS185") {
assert_eq!(record.with_abundance(), false)
} else {
assert_eq!(record.with_abundance(), true)
}
}
}
}
4 changes: 2 additions & 2 deletions src/core/src/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ impl Selection {

pub fn from_record(row: &Record) -> Result<Self> {
Ok(Self {
ksize: Some(*row.ksize()),
abund: Some(*row.with_abundance()),
ksize: Some(row.ksize()),
abund: Some(row.with_abundance()),
moltype: Some(row.moltype()),
num: None,
scaled: None,
Expand Down
Loading