Skip to content
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

Port the tables and trees sections of the tskit tut #75

Merged
merged 4 commits into from
Jun 21, 2021

Conversation

hyanwong
Copy link
Member

@hyanwong hyanwong commented Jun 16, 2021

Part of tskit-dev/tskit#1121, this moves a substantial section of what was previously the "tutorial" section into https://tskit.dev/tutorials/data_structures.html (see #36).

I haven't made much of an attempt to tidy up the flow, so it's a bit of a rag-bag at the moment, although it does roughly fit into the framework that @jeromekelleher suggested of of "Tables" then "Trees". Perhaps he could look over to see what needs moving elsewhere (and add suggestions of where to move it).

I suspect we should massage this to add stuff from https://tskit.dev/tskit/docs/stable/data-model.html, but this is hopefully a good start. It's all in jupyterbook format anyway.

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.

Good to port some of this stuff across, but I wonder if we'll be porting a lot of it back into the tskit docs when they are udpated to use the msprime-1.0 type structure.

But as you say, good to get it into JupyterBook format anyway, easy enough to port back.

We should probably remove the corresponding bits from the tskit docs, so we can keep track of what content is where (I guess putting links in to here in the relevant sections, for now?)

data/examples.py Outdated Show resolved Hide resolved
data_structures.md Outdated Show resolved Hide resolved
data_structures.md Show resolved Hide resolved
data_structures.md Outdated Show resolved Hide resolved
@hyanwong
Copy link
Member Author

Good to port some of this stuff across, but I wonder if we'll be porting a lot of it back into the tskit docs when they are udpated to use the msprime-1.0 type structure.

Possibly - but all this stuff is in the "tutorial", and I think we probably want to remove that entire page from the tskit docs and port it into separate tutorials. As we do this, we can see what should go back into the main tskit documentation, and where.

We should probably remove the corresponding bits from the tskit docs, so we can keep track of what content is where (I guess putting links in to here in the relevant sections, for now?)

I think first we merge these, then when we update the docs to remove them from tskit, we can change any dead refs to point to the tutorial section links instead. Since this is all in the tutorial.html file, which we are removing anyway, it should be easy to see what needs removing after this set of tutorial PRs are merged.

@benjeffery
Copy link
Member

Ping me when you've done JK's comments and I'll have a look over

@hyanwong
Copy link
Member Author

Ping me when you've done JK's comments and I'll have a look over

I'm thinking that we might want to remove the sections on (a) iterating through trees in a ts and (b) traversing in various ways through a single tree.

(a) can probably be (mostly) incorporated into the "getting started" tut, which has a section on the .trees() iterator, but which doesn't mention using reversed (easy to stick in) or the problems of storing trees as a list (could add as a {note})`
(b) could probably go in its own tute: e.g. "tree traversal", perhaps under a new section of tuts labelled "Analysis" which could also contain the "statistical analysis" tut?

I'm not sure whether to do this now, before you look over it @benjeffery ?

@benjeffery
Copy link
Member

These suggestions seem good, and worth doing now.

"The problems of storing trees as a list" can be repeated - it can be a real gotcha for newcomers.

@hyanwong hyanwong force-pushed the tables branch 3 times, most recently from c09e93e to f2001dc Compare June 18, 2021 16:26
@hyanwong
Copy link
Member Author

hyanwong commented Jun 18, 2021

I hope I've addressed all the issues raised, so it's probably ready for review. The singleton deletion code uses the dataclass.replace method to set the site id: I worry a little that this is a bit opaque for a new user?

site_id = tables.sites.append(site)
mut = dataclasses.replace(mut, site=site_id)  # set the new site id
tables.mutations.append(mut)

(see tskit-dev/tskit#1503 for a possible solution)

@hyanwong hyanwong force-pushed the tables branch 2 times, most recently from ed38ffd to 7055687 Compare June 18, 2021 17:47
@hyanwong
Copy link
Member Author

I kept a small section in there about "trees", as @jeromekelleher had a section titled this in the stub. But it feels a bit lonely. If it were to be removed, the entire tube is about tables & editing, so perhaps the name could be changed to, well, "Tables & editing" or something similar. I also wonder if it should go a bit lower in the list of chapters?

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 - I think we can merge and then start thinking again about where various bits of content should end up. Great to get the basic stuff ported over now.

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.

Looks great @hyanwong I've been through closely and checked links.

getting_started.md Show resolved Hide resolved
@jeromekelleher
Copy link
Member

Can you rebase here please @hyanwong, we're touching a lot of files so good to be up to date with base.

@hyanwong
Copy link
Member Author

Can you rebase here please @hyanwong, we're touching a lot of files so good to be up to date with base.

Yes, of course - rebased and force pushed. Thanks for seeing to all this. Good to get off my plate.

@jeromekelleher jeromekelleher merged commit d19e54d into tskit-dev:main Jun 21, 2021
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.

3 participants