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] cleanup and commenting of test_index.py tests. #1898

Merged
merged 46 commits into from
Mar 29, 2022
Merged

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Mar 26, 2022

Note: PR into #1891. Review & merge after that one.

This PR does some cleanup and commenting on tests/test_index.py, as well as pyflakes-based cleaning of src/sourmash/index/__init__.py.

This is a pure commenting-and-docstring PR, no code changes.

@ctb ctb changed the title [MRG] cleanup and commenting of test_index.py tests. [WIP] cleanup and commenting of test_index.py tests. Mar 26, 2022
@codecov
Copy link

codecov bot commented Mar 26, 2022

Codecov Report

Merging #1898 (841ebf1) into latest (f05e4bd) will increase coverage by 7.89%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           latest    #1898      +/-   ##
==========================================
+ Coverage   82.83%   90.73%   +7.89%     
==========================================
  Files         122       92      -30     
  Lines       13257     9052    -4205     
  Branches     1789     1789              
==========================================
- Hits        10981     8213    -2768     
+ Misses       2013      576    -1437     
  Partials      263      263              
Flag Coverage Δ
python 90.73% <ø> (-0.01%) ⬇️
rust ?

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

Impacted Files Coverage Δ
src/sourmash/index/__init__.py 96.73% <ø> (-0.02%) ⬇️
src/sourmash/index/revindex.py 63.46% <ø> (ø)
src/core/src/sketch/hyperloglog/estimators.rs
src/core/src/ffi/hyperloglog.rs
src/core/src/sketch/hyperloglog/mod.rs
src/core/src/index/revindex.rs
src/core/src/errors.rs
src/core/src/lib.rs
src/core/src/ffi/signature.rs
src/core/src/from.rs
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f05e4bd...841ebf1. Read the comment docs.

@ctb ctb changed the base branch from latest to add/manifestindex March 26, 2022 15:07
@luizirber
Copy link
Member

as well as pyflakes-based cleaning

😮 you're using pyflakes?!?!?

@ctb
Copy link
Contributor Author

ctb commented Mar 27, 2022

as well as pyflakes-based cleaning

😮 you're using pyflakes?!?!?

yes... is that surprising? or bad?

@luizirber
Copy link
Member

yes... is that surprising? or bad?

it's great, means that we might finally uncomment flake8 from the pre-commit checks 🙃

(or at least run these things more frequently and reduce the mountain of warnings 🙈 )

@ctb
Copy link
Contributor Author

ctb commented Mar 27, 2022

yes... is that surprising? or bad?

it's great, means that we might finally uncomment flake8 from the pre-commit checks 🙃

(or at least run these things more frequently and reduce the mountain of warnings 🙈 )

sure - I can make that an issue. It'd be good to do it systematically across the code base; in the tests I have more tolerance for unused variables, but even there flakes found a number of shadowed tests - see #1863.

@ctb ctb changed the title [WIP] cleanup and commenting of test_index.py tests. [MRG] cleanup and commenting of test_index.py tests. Mar 27, 2022
Base automatically changed from add/manifestindex to latest March 28, 2022 23:36
@ctb ctb merged commit c26ac4e into latest Mar 29, 2022
@ctb ctb deleted the cleanup/index branch March 29, 2022 00:01
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