Skip to content

Conversation

@benjeffery
Copy link
Member

Resolves #534
As we are doing a metadata docs pass in #603 I have only done minimal docs.

@benjeffery benjeffery requested a review from jeromekelleher June 3, 2020 11:03
@codecov
Copy link

codecov bot commented Jun 3, 2020

Codecov Report

Merging #666 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #666      +/-   ##
==========================================
+ Coverage   87.74%   87.75%   +0.01%     
==========================================
  Files          23       23              
  Lines       17668    17687      +19     
  Branches     3498     3498              
==========================================
+ Hits        15503    15522      +19     
  Misses       1062     1062              
  Partials     1103     1103              
Flag Coverage Δ
#c_tests 89.03% <100.00%> (+0.01%) ⬆️
#python_c_tests 91.23% <100.00%> (+0.01%) ⬆️
#python_tests 98.99% <100.00%> (+<0.01%) ⬆️
Impacted Files Coverage Δ
python/tskit/tables.py 99.66% <100.00%> (+<0.01%) ⬆️
python/tskit/trees.py 98.21% <100.00%> (+<0.01%) ⬆️

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 5958f0e...f96a4db. Read the comment docs.

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, minor comments.

)

def metadata_example_data(self):
try:
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this as it makes the tests stateful and the test data will depend on what other tests are run on this class. Also adding instance variables to a TestCase is a bit weird. Either put in some constants, or make val a parameter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it a param.

@benjeffery benjeffery force-pushed the top-level-metadata-python branch from 8eab997 to 606a1df Compare June 3, 2020 12:13
@benjeffery
Copy link
Member Author

@jeromekelleher Fixed up - given the number of this PR I got off lightly. 👿

@jeromekelleher
Copy link
Member

Not an Iron Maiden fan then? I keep singing "6,66 the PR of the Beast" to myself. Classic.

@benjeffery benjeffery force-pushed the top-level-metadata-python branch from 579299d to f96a4db Compare June 3, 2020 14:01
@benjeffery
Copy link
Member Author

Travis AWOL again - will look into this as happening a lot.

@mergify mergify bot merged commit 5da361b into tskit-dev:master Jun 3, 2020
@benjeffery benjeffery deleted the top-level-metadata-python branch June 3, 2020 20:55
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.

Tree-sequence-level metadata

2 participants