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: prepare to remove sourmash compute for sourmash v5.0 #3103

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Apr 1, 2024

This PR refactors code to eliminate any internal dependencies on the compute command or codebase, in preparation for removing sourmash compute in v5.0, per #1286.

Specifically, this PR:

  • shifts common sketching code from src/sourmash/command_compute.py to src/sourmash/command_sketch.py;
  • refactors three tests that were still using sourmash compute to use sourmash sketch instead.

No functionality is altered or adjusted in this PR; it's all just code refactoring.

The next step for #1286 would maybe be adjust the code and the tests to respect the --v4 and --v5 flags (as used in #3072 / #3074). The actual compute code and command wouldn't be removed until after a 5.0 release, I think.

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 89.18919% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 86.55%. Comparing base (e0d002a) to head (34025f3).

Files Patch % Lines
src/sourmash/command_sketch.py 89.14% 21 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           latest    #3103      +/-   ##
==========================================
- Coverage   86.90%   86.55%   -0.36%     
==========================================
  Files         136      136              
  Lines       15554    15809     +255     
  Branches     2640     2713      +73     
==========================================
+ Hits        13517    13683     +166     
- Misses       1735     1816      +81     
- Partials      302      310       +8     
Flag Coverage Δ
hypothesis-py 25.36% <23.55%> (-0.07%) ⬇️
python 92.32% <89.18%> (-0.56%) ⬇️
rust 61.47% <ø> (ø)

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.

@ctb ctb changed the title EXP: remove sourmash compute for sourmash v5.0 WIP: prepare to remove sourmash compute for sourmash v5.0 Apr 1, 2024
@ctb ctb changed the title WIP: prepare to remove sourmash compute for sourmash v5.0 MRG: prepare to remove sourmash compute for sourmash v5.0 Apr 1, 2024
@ctb
Copy link
Contributor Author

ctb commented Apr 1, 2024

Ready for review & merge!

Copy link
Contributor

@bluegenes bluegenes left a comment

Choose a reason for hiding this comment

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

lgtm!

@ctb ctb merged commit f4e7205 into latest Apr 12, 2024
39 of 41 checks passed
@ctb ctb deleted the remove_compute_v5 branch April 12, 2024 18:44
@ctb ctb mentioned this pull request Jun 10, 2024
ctb added a commit that referenced this pull request Jun 11, 2024
Minor new features:

* add `--set-name` to `sig intersect` and `sig subtract` (#3162)
* upgrade `sig overlap` and `sig subtract` to load more than JSON
signatures (#3153)
* force continue past `tax genome` classification errors (#3100)

Bug fixes:

* fix `remaining_bp` output from sourmash gather (#3195)
* fix RocksDB-based gather & other rust-based infelicities revealed by
plugins (#3193, #3197)
* use correct denominator in f_unique_to_query (#3138)

Cleanup and documentation updates:

* update JOSS for sourmash v4 (#3114, #3203, #3209)
* fix links to taxonomy spreadsheets (#3119)
* fix description of `f_unique_weighted` (#3164)

Developer updates:

* transition internal signature loading functions (#3161)
* allow get/set record.filename (#3121)
* round a number that is losing precision in 15th place in
`test_distance_utpy` (#3126)
* disable ppc64le wheel building (#3127)
* prepare to remove `sourmash compute` for sourmash v5.0 (#3103)
* add rustup target x86_64-apple-darwin (#3148)
* mv `.cargo/config` to `config.toml` (#3147)
* fix clippy warnings about max_value (#3146)
* bump to v4.8.9-dev (#3135)
* update src/core/CHANGELOG.md for sourmash-rs core release r0.14.0
(#3199)

Dependabot updates:

* Bump DeterminateSystems/nix-installer-action from 11 to 12 (#3184)
* Bump DeterminateSystems/magic-nix-cache-action from 6 to 7 (#3185)
* Bump statrs from 0.16.0 to 0.16.1 (#3186)
* Bump serde from 1.0.202 to 1.0.203 (#3175)
* Bump ouroboros from 0.18.3 to 0.18.4 (#3176)
* Bump itertools from 0.12.1 to 0.13.0 (#3166)
* Bump camino from 1.1.6 to 1.1.7 (#3169)
* Bump serde from 1.0.201 to 1.0.202 (#3168)
* Bump thiserror from 1.0.60 to 1.0.61 (#3167)
* Bump pypa/cibuildwheel from 2.18.0 to 2.18.1 (#3165)
* Bump DeterminateSystems/magic-nix-cache-action from 4 to 6 (#3157)
* Bump DeterminateSystems/nix-installer-action from 10 to 11 (#3156)
* Bump pypa/cibuildwheel from 2.17.0 to 2.18.0 (#3155)
* Bump serde_json from 1.0.116 to 1.0.117 (#3159)
* Bump thiserror from 1.0.59 to 1.0.60 (#3158)
* Bump serde from 1.0.200 to 1.0.201 (#3160)
* Bump roaring from 0.10.3 to 0.10.4 (#3142)
* Bump histogram from 0.10.0 to 0.10.1 (#3141)
* Bump getrandom from 0.2.14 to 0.2.15 (#3143)
* Bump num-iter from 0.1.44 to 0.1.45 (#3140)
* Bump jinja2 from 3.1.3 to 3.1.4 (#3145)
* Bump serde from 1.0.199 to 1.0.200 (#3144)
* Bump serde from 1.0.198 to 1.0.199 (#3130)
* Bump conda-incubator/setup-miniconda from 3.0.3 to 3.0.4 (#3131)
* Update pytest requirement from <8.2.0,>=6.2.4 to >=6.2.4,<8.3.0
(#3132)
* Bump myst-parser from 2.0.0 to 3.0.1 (#3133)
* Bump thiserror from 1.0.58 to 1.0.59 (#3123)
* Bump serde_json from 1.0.115 to 1.0.116 (#3124)
* Bump serde from 1.0.197 to 1.0.198 (#3122)
* Update docutils requirement from <0.21,>=0.17.1 to >=0.17.1,<0.22
(#3116)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants