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

Experimental v5 SBT format #694

Merged
merged 1 commit into from
Sep 27, 2019
Merged

Experimental v5 SBT format #694

merged 1 commit into from
Sep 27, 2019

Conversation

luizirber
Copy link
Member

@luizirber luizirber commented Jul 8, 2019

This brings the minimum v5 support from #456 (internal changes and loading from file), while still defaulting to saving to v4. This allows experimenting with the format without commiting to full support (yet).

Differences from v4 to v5:

  • Internal representation uses two dictionaries: one for internal nodes, another for leaves.
  • Only leaves are required to be present in the SBT file, and it will rebuild internal nodes on load.
  • Prototype for insertion without updating internal nodes (delay internal node generation to save)

(v5 is the default in the rust codebase)

Checklist

  • 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
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

Merging #694 into master will decrease coverage by 0.09%.
The diff coverage is 89.94%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #694     +/-   ##
=========================================
- Coverage   88.71%   88.61%   -0.1%     
=========================================
  Files          30       30             
  Lines        4617     4735    +118     
  Branches       45       45             
=========================================
+ Hits         4096     4196    +100     
- Misses        519      537     +18     
  Partials        2        2
Impacted Files Coverage Δ
sourmash/sbt.py 85.53% <89.94%> (-0.19%) ⬇️

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 878540f...98aa5b5. Read the comment docs.

@luizirber luizirber force-pushed the feature/v5_loading branch 2 times, most recently from a002aa5 to 02af7c1 Compare July 9, 2019 05:00
make nodes and missing nodes 'private'
save to v4, but keep _save_v5 around
@luizirber
Copy link
Member Author

Ready for review and merge @ctb

@luizirber luizirber requested a review from ctb September 27, 2019 02:09
@ctb
Copy link
Contributor

ctb commented Sep 27, 2019

Go ahead and merge as you wish @luizirber !

1 similar comment
@ctb
Copy link
Contributor

ctb commented Sep 27, 2019

Go ahead and merge as you wish @luizirber !

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