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.

Sign up for GitHub

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

Jump to bottom

Use dict representation for compress and decompress #42

Merged
merged 1 commit into from
Apr 22, 2021
Merged

Use dict representation for compress and decompress #42

merged 1 commit into from
Apr 22, 2021

Conversation

benjeffery
Copy link
Member

Fixes #39
Fixes #38

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2021 •
edited
Loading

Codecov Report

Merging #42 (dc770d3) into main (55e7fb4) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   97.57%   97.70%   +0.12%     
==========================================
  Files           6        6              
  Lines         289      305      +16     
  Branches       42       55      +13     
==========================================
+ Hits          282      298      +16     
  Misses          5        5              
  Partials        2        2
Impacted Files Coverage Δ
tszip/compression.py 98.88% <100.00%> (+0.10%) ⬆️

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 55e7fb4...dc770d3. Read the comment docs.

@tskit-dev tskit-dev deleted a comment from pep8speaks Apr 21, 2021
jeromekelleher
jeromekelleher approved these changes Apr 21, 2021
View reviewed changes
Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

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

LGTM!

tests/test_compression.py Outdated Show resolved Hide resolved
tests/test_cli.py
@@ -43,6 +42,8 @@ class TestException(Exception):
Custom exception we can throw for testing.
"""

__test__ = False
Copy link
Member

Choose a reason for hiding this comment

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

What's happening here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Telling pytest that isn't a test class. Added a comment.

@jeromekelleher
Copy link
Member

We should update the CHANGELOG with a summary of the changes. So the idea is that the tszip encoding should be identical to older versions?

Is there any way we can verify this? Maybe we should put in a few old tszipped files in repo, and verify that we recover these correctly, at least?

@benjeffery
Copy link
Member Author

I've added a changelog and a test of a legacy file. The idea is that old files should still work.

@mergify mergify bot merged commit ca42f1f into tskit-dev:main Apr 22, 2021
@benjeffery benjeffery deleted the fix_new_columns branch April 22, 2021 22:39
@brianzhang01 brianzhang01 mentioned this pull request Jun 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeromekelleher jeromekelleher jeromekelleher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor to use the dict representation of tree sequence Add tests for metadata schemas across tables
3 participants
@benjeffery @codecov-commenter @jeromekelleher