Skip to content

First pass at changing .length to .span #169

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

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

hyanwong
Copy link
Member

@hyanwong hyanwong commented Apr 5, 2019

Closes #29

@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #169 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage   85.95%   85.95%   +<.01%     
==========================================
  Files          16       16              
  Lines        8705     8707       +2     
  Branches     1670     1670              
==========================================
+ Hits         7482     7484       +2     
  Misses        732      732              
  Partials      491      491
Flag Coverage Δ
#c_tests 85.95% <100%> (ø) ⬆️
#python_tests 98.26% <100%> (ø) ⬆️
Impacted Files Coverage Δ
python/tskit/trees.py 98.59% <100%> (ø) ⬆️

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 6e411ff...77b9259. Read the comment docs.

@jeromekelleher
Copy link
Member

jeromekelleher commented Apr 5, 2019

Looks good, needs some tests though. Just need to ensure that length == span == (right - left) somewhere.

@hyanwong
Copy link
Member Author

hyanwong commented Apr 5, 2019

Done, shall I squash?

:attr:`.length`, which describes the length of the interval
covered by the tree in genomic coordinates.
(note that this is not related to the deprecated property :attr:`.length`
which is a deprecated alias for the genomic :attr:`.span` covered by a tree)
Copy link
Member

Choose a reason for hiding this comment

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

deprecated twice here.

@@ -1407,6 +1407,9 @@ def test_deprecated_apis(self):
self.assertEqual(
ts.get_pairwise_diversity(samples), ts.pairwise_diversity(samples))
self.assertTrue(np.array_equal(ts.get_samples(), ts.samples()))
# Test APIs for a single tree in a ts
self.assertEqual(ts.first().get_length, ts.first().span)
self.assertEqual(ts.first().length, ts.first().span)
Copy link
Member

Choose a reason for hiding this comment

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

Be nice to check this somewhere on on not just the first tree. Also, check

l, r = tree.interval
tree.span == r - 

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep - I've just re-written it (and put it in a more sensible place). Thanks.

@jeromekelleher
Copy link
Member

OK looks good, can you squash please? (The git commit --amend option is useful for this sort of thing...)

@hyanwong
Copy link
Member Author

hyanwong commented Apr 5, 2019

Done. Thanks for the tip. Will look into it

@jeromekelleher
Copy link
Member

Thanks @hyanwong, merging.

@jeromekelleher jeromekelleher merged commit bebe3c1 into tskit-dev:master Apr 8, 2019
@hyanwong hyanwong deleted the span-not-length branch September 24, 2019 18:11
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