Skip to content

Conversation

@benjeffery
Copy link
Member

C API part of #534. Adds metadata and metadata_schema fields to table collection, with accessors on tree sequence. These store arbitrary bytes and are optional in the file format.

@benjeffery benjeffery requested a review from jeromekelleher May 24, 2020 14:03
@benjeffery benjeffery force-pushed the top-level-metadata branch from 3ddec5e to 76a2117 Compare May 24, 2020 14:04
@benjeffery benjeffery force-pushed the top-level-metadata branch 5 times, most recently from 328791d to 16efd3b Compare May 24, 2020 15:57
@codecov
Copy link

codecov bot commented May 24, 2020

Codecov Report

Merging #642 into master will decrease coverage by 0.08%.
The diff coverage is 65.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
- Coverage   87.71%   87.62%   -0.09%     
==========================================
  Files          23       23              
  Lines       17360    17426      +66     
  Branches     3421     3432      +11     
==========================================
+ Hits        15227    15270      +43     
- Misses       1044     1058      +14     
- Partials     1089     1098       +9     
Flag Coverage Δ
#c_tests 88.86% <65.67%> (-0.13%) ⬇️
#python_c_tests 91.08% <ø> (ø)
#python_tests 98.90% <ø> (ø)
Impacted Files Coverage Δ
c/tskit/tables.c 79.07% <61.01%> (-0.26%) ⬇️
c/tskit/trees.c 90.66% <100.00%> (+0.02%) ⬆️

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 1a1f315...8a49681. Read the comment docs.

@benjeffery
Copy link
Member Author

Looks like I could get a little more coverage here, setting back to draft.

@benjeffery benjeffery marked this pull request as draft May 24, 2020 16:30
@benjeffery benjeffery force-pushed the top-level-metadata branch 2 times, most recently from 7c8f775 to 4235fbe Compare May 25, 2020 01:12
@benjeffery
Copy link
Member Author

benjeffery commented May 25, 2020

@jeromekelleher Added a test on the TS methods. Looks like the rest of the coverage misses are hard error paths. Ready for review.

@benjeffery benjeffery marked this pull request as ready for review May 25, 2020 01:19
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, thanks @benjeffery. I'll update to fix a couple of small issues and tag for merging.

@benjeffery
Copy link
Member Author

@jeromekelleher did you see my comment above about the casting to char?

@benjeffery
Copy link
Member Author

Oops just seen the reply, ignore!

@jeromekelleher
Copy link
Member

I seem to have messed the PR up here - I thought you were off today @benjeffery so I force pushed a couple of changes. Looks like I broke stuff.

Crap, I think I pulled in kastore updates by git commit -a.

@jeromekelleher
Copy link
Member

I made the mess, I'll fix it up!

@benjeffery
Copy link
Member Author

I'm off, but can quickly push the branch back to how it was?

@jeromekelleher
Copy link
Member

I'm off, but can quickly push the branch back to how it was?

Na, thanks, it's an easy fix.

@benjeffery benjeffery force-pushed the top-level-metadata branch from 3673e8f to 8a49681 Compare May 25, 2020 15:26
@mergify mergify bot merged commit 623b3b3 into tskit-dev:master May 25, 2020
@benjeffery benjeffery deleted the top-level-metadata branch May 30, 2020 23:45
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