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] Add some more doctests for API definition. #436

Merged
merged 17 commits into from
Dec 28, 2018
Merged

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Mar 9, 2018

Fixes #329 #147 by adding lots of API documentation.

See expanded docs here.

Also fixes #580 (database scaled values) 'cause why not.

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@codecov-io
Copy link

codecov-io commented Mar 9, 2018

Codecov Report

Merging #436 into master will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #436     +/-   ##
=========================================
+ Coverage   88.53%   88.64%   +0.1%     
=========================================
  Files          25       25             
  Lines        3786     3786             
  Branches       37       37             
=========================================
+ Hits         3352     3356      +4     
+ Misses        434      430      -4
Impacted Files Coverage Δ
sourmash/signature.py 95.34% <0%> (+0.58%) ⬆️
sourmash/sbtmh.py 85.29% <0%> (+2.2%) ⬆️

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 99e86bb...c944b4e. Read the comment docs.

@ctb ctb changed the title Add some more doctests for API definition. [WIP] Add some more doctests for API definition. Mar 10, 2018
@ctb
Copy link
Contributor Author

ctb commented Dec 27, 2018

I think another pass is needed to adjust the language better in the num vs scaled section, but otherwise I like. @luizirber any thoughts?

@ctb ctb changed the title [WIP] Add some more doctests for API definition. [MRG] Add some more doctests for API definition. Dec 27, 2018
@ctb
Copy link
Contributor Author

ctb commented Dec 27, 2018

I've decided I like it. @luizirber ready for review and merge? :)

Copy link
Member

@luizirber luizirber left a comment

Choose a reason for hiding this comment

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

I think it does a good job at covering what we have nowadays.

What bothers me is that we still conflate sigs and minhashes sometimes (especially the load_one_signature function), but that will only be fixable in sourmash 3.0...

doc/api-example.md Outdated Show resolved Hide resolved
@ctb ctb merged commit 858e7e0 into master Dec 28, 2018
@ctb ctb deleted the add/more_doctests branch December 28, 2018 15:30
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.

update documentation to make clear the scaled differences between LCA and SBT More API docs.
3 participants