tskit icon indicating copy to clipboard operation
tskit copied to clipboard

Add 'mid' property to Interval class

Open currocam opened this issue 1 year ago • 10 comments

Description

Add mid property to the Interval and Tree class, as requested in #2903

PR Checklist:

  • [x] Tests that fully cover new/changed functionality.
  • [x] Documentation including tutorial content if appropriate.
  • [x] Changelogs, if there are API changes.

currocam avatar Jun 18 '24 17:06 currocam

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.72%. Comparing base (a0baaaf) to head (b023b11). Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2960   +/-   ##
=======================================
  Coverage   89.72%   89.72%           
=======================================
  Files          29       29           
  Lines       31567    31573    +6     
  Branches     6113     6115    +2     
=======================================
+ Hits        28322    28328    +6     
  Misses       1853     1853           
  Partials     1392     1392           
Flag Coverage Δ
c-tests 86.55% <ø> (ø)
lwt-tests 80.78% <ø> (ø)
python-c-tests 88.98% <ø> (ø)
python-tests 99.01% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
python/tskit/trees.py 98.78% <100.00%> (+<0.01%) :arrow_up:

codecov[bot] avatar Jun 18 '24 17:06 codecov[bot]

ci/circleci: build-32 and codecov/project don't pass, but I don't know how those errors are related with my PR

currocam avatar Jun 18 '24 17:06 currocam

Hm - well, there's currently just one test, but two methods added (although one calls the other). Perhaps add another test for the Interval property (so it is tested autonomously)? The CI failures might be just transient.

petrelharp avatar Jun 18 '24 19:06 petrelharp

Hi, I just added the second test! Thank you for reviewing the changes

And for making such amazing piece of software! I've used a lot doing my master's thesis and I hope I will keep doing after I finish :)

currocam avatar Jun 19 '24 11:06 currocam

Looks like you have a few lint issues that need to be fixed also.

jeromekelleher avatar Jun 20 '24 09:06 jeromekelleher

Thanks @currocam! It's probably worth installing pre-commit, following the dev docs here. (pre-commit install, should do it).

It would be good to rebase and squash the commits also, as this is a small change. This guide may be helpful.

jeromekelleher avatar Jun 21 '24 08:06 jeromekelleher

Hi! Thank you very much! I was in fact in the process of install pre-commit, thanks for pointing that out!

currocam avatar Jun 21 '24 08:06 currocam

Great, looks ready to merge 👍

jeromekelleher avatar Jun 21 '24 08:06 jeromekelleher

@mergifyio rebase

benjeffery avatar Jun 27 '24 15:06 benjeffery

rebase

☑️ Nothing to do

  • [ ] -conflict [📌 rebase requirement]
  • [X] -closed [📌 rebase requirement]
  • [X] queue-position=-1 [📌 rebase requirement]
  • [X] any of:
    • [X] #commits-behind>0 [📌 rebase requirement]
    • [ ] #commits>1 [📌 rebase requirement]
    • [ ] -linear-history [📌 rebase requirement]

mergify[bot] avatar Jun 27 '24 15:06 mergify[bot]