Skip to content

Port over intervals #2678

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
Merged
merged 5 commits into from
Jan 13, 2023
Merged

Port over intervals #2678

merged 5 commits into from
Jan 13, 2023

Conversation

jeromekelleher
Copy link
Member

Updated version of #2636 with care taken to have full information about code provenance.

@jeromekelleher jeromekelleher mentioned this pull request Jan 10, 2023
Closed
@jeromekelleher jeromekelleher requested review from hyanwong, benjeffery and gtsambos and removed request for hyanwong and benjeffery January 10, 2023 14:39
@jeromekelleher
Copy link
Member Author

I've basically just done some cherry picking from your commits @hyanwong, so it should be the same as #2636 (including your email in the Git log for the commits)

@jeromekelleher jeromekelleher requested review from benjeffery and removed request for gtsambos January 10, 2023 14:44
hyanwong
hyanwong approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@hyanwong hyanwong left a comment

Choose a reason for hiding this comment

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

Thanks. That looks great. One minor comment

@codecov
Copy link

codecov bot commented Jan 10, 2023 •
edited
Loading

Codecov Report

Merging #2678 (a615116) into main (a615116) will not change coverage.
The diff coverage is n/a.

❗ Current head a615116 differs from pull request most recent head ccefea6. Consider uploading reports for the commit ccefea6 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2678   +/-   ##
=======================================
  Coverage   93.94%   93.94%           
=======================================
  Files          28       28           
  Lines       27823    27823           
  Branches     1528     1528           
=======================================
  Hits        26139    26139           
  Misses       1648     1648           
  Partials       36       36
Flag Coverage Δ
c-tests 92.26% <0.00%> (ø)
lwt-tests 89.05% <0.00%> (ø)
python-c-tests 71.90% <0.00%> (ø)
python-tests 98.98% <0.00%> (ø)

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

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 1fc9e56...ccefea6. Read the comment docs.

@hyanwong
Copy link
Member

Do we want the documentation to appear in tskit now, rather than msprime? I assume so. I can sort that later if you like @jeromekelleher ?

@jeromekelleher
Copy link
Member Author

Let's file an issue and follow up?

@hyanwong
Copy link
Member

hyanwong commented Jan 10, 2023 •
edited
Loading

Let's file an issue ...

#2679

benjeffery
benjeffery approved these changes Jan 10, 2023
View reviewed changes
Copy link
Member

@benjeffery benjeffery left a comment

Choose a reason for hiding this comment

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

LGTM, one nit about the docstring. Thanks for factoring out truncate_rows I should have done that with the original work!

@jeromekelleher jeromekelleher added the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Jan 11, 2023
@jeromekelleher
Copy link
Member Author

Huh, no idea what this docs build failure is about - have you seen this one before @benjeffery?

@benjeffery
Copy link
Member

Huh, no idea what this docs build failure is about - have you seen this one before @benjeffery?

Yep, first happened last night and is fixed in the Py3.11 PR that I'm merging imminently when codecov want to work.

@benjeffery
Copy link
Member

Rebased and conflicts fixed.

jeromekelleher and others added 5 commits January 13, 2023 01:06
@jeromekelleher @mergify
CODE IMPORT: copy intervals code from msprime
3b2ba7d 
The authors of this module and associated tests have agreed to release
the code originally developed for the msprime package under the MIT
licence (see tskit-dev#2636). Specifically:

@jeromekelleher: "I approve the relicensing of this code originally
developed in msprime from GPLv3 to MIT."

@hyanwong: "I am happy to relicense all of my contributions to this code
(originally developed in msprime) from GPLv3 to MIT."

@awohns: "I also approve the relicensing of this code originally
developed in msprime from GPLv3 to MIT."

@grahamgower: "I approve the relicensing of this code originally
developed in msprime from GPLv3 to MIT."

@petrelharp: "I approve the license change!"
@jeromekelleher @mergify
Update copyright headers and licence text
d3dd3b3
@hyanwong @mergify
Allow alignments to be specified
6c7632d 
To match the msprime `text_table` function
@hyanwong @mergify
Place html table formatting in util.py
4be6206 
Allows it to be used by other tabular outputting routines.

Also put the "limit" functionality into a single util function
@hyanwong @mergify
Fixup intervals module
ccefea6 
Rework a little to use the built-in tskit unicode and html output functions
@benjeffery
Copy link
Member

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jan 13, 2023

rebase

✅ Nothing to do for rebase action

@benjeffery
Copy link
Member

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jan 13, 2023

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit 805b153 into tskit-dev:main Jan 13, 2023
@mergify mergify bot removed the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Jan 13, 2023
@jeromekelleher jeromekelleher deleted the port-over-intervals branch January 13, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyanwong hyanwong hyanwong approved these changes

@benjeffery benjeffery benjeffery 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.
3 participants
@jeromekelleher @hyanwong @benjeffery