MRG: make core Manifest booleans python compatible (core) #3007

bluegenes · 2024-02-16T21:20:12Z

rust bools are true/false, python bools are True/False.
When we read in a manifest csv in python (e.g. for sourmash sig summarize), we use ast.literal_eval, which does not consider true/false valid bools.

This PR adds custom serialization manifest::intbool to write 0/1 instead of string booleans . This allows us to read manifests written here from python. Alternatively, we could useTrue/False.

for k in boolrows:
    row[k] = bool(ast.literal_eval(str(row[k])))

sourmash/src/sourmash/manifest.py

Lines 92 to 93 in f3dc5f2

    
           for k in boolrows: 
        
               row[k] = bool(ast.literal_eval(str(row[k])))

Problem arises when using core manifest utils in branchwater manysketch
sourmash-bio/sourmash_plugin_branchwater#217
sourmash-bio/sourmash_plugin_branchwater#203

codecov · 2024-02-16T21:28:30Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (1ed81fe) 86.63% compared to head (1ab7998) 86.62%.

Files	Patch %	Lines
src/core/src/manifest.rs	37.50%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest    #3007      +/-   ##
==========================================
- Coverage   86.63%   86.62%   -0.02%     
==========================================
  Files         135      135              
  Lines       15389    15392       +3     
  Branches     2624     2624              
==========================================
  Hits        13333    13333              
- Misses       1755     1758       +3     
  Partials      301      301

Flag	Coverage Δ
hypothesis-py	`25.56% <ø> (ø)`
python	`92.85% <ø> (ø)`
rust	`59.37% <37.50%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

luizirber · 2024-02-17T05:23:10Z

src/core/src/manifest.rs

@@ -26,7 +26,7 @@ pub struct Record {

    md5short: String,

-    #[getset(get = "pub", set = "pub")]
+    #[getset(get_copy = "pub", set = "pub")]


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

) Updates `src/core/CHANGELOG.md` with release notes; see below. ## Release checklist from #2987 (comment): - [x] verify minimum supported rust version (MSRV) for writing release notes (see #2988 for an example); MSRV is checked by CI in `.github/workflows/rust.yml`, `minimum_rust_version` - [x] write release notes using `git log --oneline r0.12.0..latest src/core | cut -d\ -f2- > /tmp/out.txt` - [x] verify that the ChangeLog is up to date: https://github.com/sourmash-bio/sourmash/blob/latest/src/core/CHANGELOG.md - [x] bump version in `src/core/Cargo.toml` ## Release notes for r0.13.0 MSRV: 1.65 Changes/additions: * Calculate all gather stats in rust; use for rocksdb gather (#2943) * adjust protein ksize for record/manifest (#3019) * Allow changing storage location for a collection in RevIndex (#3015) * make core Manifest booleans python compatible (core) (#3007) Updates: * Bump roaring from 0.10.2 to 0.10.3 (#3014) * Bump histogram from 0.9.0 to 0.9.1 (#3002) * Bump chrono from 0.4.33 to 0.4.34 (#3000) * Bump web-sys from 0.3.67 to 0.3.68 (#2998) * Bump num-iter from 0.1.43 to 0.1.44 (#2997) * Bump wasm-bindgen-test from 0.3.40 to 0.3.41 (#2996)

This PR updates manifest generation in `manysketch` to use `Record` and `Manifest` utils from core. This also makes the code significantly simpler, since we can generate manifest information directly from the signature, rather than saving sketch parameters alongside sketches. Required `0.13.0` core due to these fixes: - sourmash-bio/sourmash#3007 - sourmash-bio/sourmash#3019 * Fixes #203

# Release notes for sourmash v4.8.7 Note: This release changes the way `sourmash multigather` names output files in some situations. Please see #2722 for details. Minor new features: * support proper manifest creation with `--relpath` for `sig check` and `sig collect` (#3054) * fix `multigather` output by adding md5sum along with `-U/--output-add-query-md5sum` (#2722) * enable loading lineages from annotated gather with match_name instead of name (#3078) Bug fixes: * fix output for `sketch ... --singleton` (#3066) * fix `calculate_gather_stats` `threshold=0` bug (#3052) Cleanup and documentation updates: * adjust protein ksize for record/manifest (#3019) * Resolve `sourmash gather --help` issue (#3032) * rework the manifest documentation; do misc cleanup (#3027) * add branchwater web to docs (#3018) Developer updates: * make core Manifest booleans python compatible (core) (#3007) * safer ksize selection while still accommodating k=k*3 (#3028) * fix clippy beta issues (#3088) * tell dependabot to ignore upgrades to `byteorder`, `chrono`, `once_cell`, and `wasm-bindgen` (#3065) * update rust changelog for r0.13.0 in preparation for release (#3033) * Allow changing storage location for a collection in RevIndex (#3015) * Fix tox and nix configs so all tox tests execute correctly (#2992) * Calculate all gather stats in rust; use for rocksdb gather (#2943) * bump screed req to 1.1.3 (#3067) * bump to v4.8.7-dev (#2989) Dependabot updates: * Bump DeterminateSystems/magic-nix-cache-action from 1 to 3 (#2994) * Bump DeterminateSystems/magic-nix-cache-action from 3 to 4 (#3085) * Bump DeterminateSystems/nix-installer-action from 4 to 9 (#2995) * Bump DeterminateSystems/nix-installer-action from 9 to 10 (#3083) * Bump chrono from 0.4.33 to 0.4.34 (#3000) * Bump conda-incubator/setup-miniconda from 3.0.1 to 3.0.2 (#3046) * Bump conda-incubator/setup-miniconda from 3.0.2 to 3.0.3 (#3057) * Bump histogram from 0.9.0 to 0.9.1 (#3002) * Bump itertools from 0.12.0 to 0.12.1 (#3043) * Bump log from 0.4.20 to 0.4.21 (#3062) * Bump num-iter from 0.1.43 to 0.1.44 (#2997) * Bump pypa/cibuildwheel from 2.16.5 to 2.17.0 (#3084) * Bump rayon from 1.8.1 to 1.9.0 (#3058) * Bump roaring from 0.10.2 to 0.10.3 (#3014) * Bump serde from 1.0.196 to 1.0.197 (#3045) * Bump serde_json from 1.0.113 to 1.0.114 (#3044) * Bump tempfile from 3.10.0 to 3.10.1 (#3059) * Bump thiserror from 1.0.56 to 1.0.57 (#2999) * Bump thiserror from 1.0.57 to 1.0.58 (#3082) * Bump wasm-bindgen from 0.2.91 to 0.2.92 (#3060) * Bump wasm-bindgen-test from 0.3.40 to 0.3.41 (#2996) * Bump wasm-bindgen-test from 0.3.41 to 0.3.42 (#3063) * Bump web-sys from 0.3.67 to 0.3.68 (#2998) * Bump web-sys from 0.3.68 to 0.3.69 (#3061) * Revert "Bump wasm-bindgen from 0.2.91 to 0.2.92 (#3060)" (#3064) * Update asv to 0.6.2 (#3025) * Update pytest requirement from <8.1.0,>=6.2.4 to >=6.2.4,<8.2.0 (#3075)

bluegenes added 2 commits February 16, 2024 13:02

write to pybool

375ddb2

rustfmt

46759e7

bluegenes mentioned this pull request Feb 16, 2024

MRG: Use core manifest utils sourmash-bio/sourmash_plugin_branchwater#217

Merged

bluegenes changed the title ~~WIP: write Manifest booleans in python format (core)~~ MRG: write Manifest booleans in python format (core) Feb 16, 2024

bluegenes and others added 2 commits February 16, 2024 17:13

use integer bool instead

296e4b4

make ksize and with_abundance getters return a copy instead of reference

bd4385a

luizirber self-requested a review February 17, 2024 05:22

luizirber reviewed Feb 17, 2024

View reviewed changes

luizirber approved these changes Feb 17, 2024

View reviewed changes

Merge branch 'latest' into pybool-record

f7de9f1

bluegenes changed the title ~~MRG: write Manifest booleans in python format (core)~~ MRG: make core Manifest booleans python compatible (core) Feb 17, 2024

Merge branch 'latest' into pybool-record

1ab7998

bluegenes merged commit 881ec36 into latest Feb 17, 2024
38 of 40 checks passed

bluegenes deleted the pybool-record branch February 17, 2024 20:04

ctb mentioned this pull request Feb 23, 2024

MRG: update rust changelog for r0.13.0 in preparation for release #3033

Merged

4 tasks

ctb mentioned this pull request Mar 21, 2024

MRG: 4.8.7 release branch #3090

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRG: make core Manifest booleans python compatible (core) #3007

MRG: make core Manifest booleans python compatible (core) #3007

bluegenes commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 16, 2024 •

edited

Loading

luizirber Feb 17, 2024

	for k in boolrows:
	row[k] = bool(ast.literal_eval(str(row[k])))

MRG: make core Manifest booleans python compatible (core) #3007

MRG: make core Manifest booleans python compatible (core) #3007

Conversation

bluegenes commented Feb 16, 2024 • edited Loading

codecov bot commented Feb 16, 2024 • edited Loading

Codecov Report

luizirber Feb 17, 2024

Choose a reason for hiding this comment

bluegenes commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 16, 2024 •

edited

Loading