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

Support for manually created tree sequences with spatial information #144

Merged
merged 9 commits into from Oct 18, 2023

Conversation

bodkan
Copy link
Owner

@bodkan bodkan commented Oct 14, 2023

While doing some follow up inference work, it turned out that slendr's ts_load() function makes an implicit assumption that the only spatial tree sequences (those which store spatial coordinates in the individual tree sequence table) are those coming from SLiM. Naturally, loading a manually created tree sequence (a tree sequence composed from individual tables) ignored spatial coordinate columns.

So, with a tree sequence created like this:

nodes <- tribble(
  ~id, ~is_sample, ~time, ~individual,
  0,   1,          0,     0,
  1,   1,          0,     1,
  2,   0,          3,     2,
  3,   1,          3,     3,
  4,   0,          6,     4
)

edges <- tribble(
  ~left, ~right, ~parent, ~child,
  0,     10,     2,       0,
  0,     10,     2,       1,
  0,     10,     4,       3,
  0,     10,     4,       2

)

individuals <- tribble(
  ~id, ~flags, ~x,  ~y,
  0,   0,      2,   4,
  1,   0,      8,   7,
  2,   0,      5,   5.5,
  3,   0,      4,   2,
  4,   0,      4.5, 3.75
)

ts <- compose_ts(nodes, edges, individuals)

calling ts_nodes(ts) on a serialized and ts_load()-ed tree sequence returns this (note the absence of coordinates of nodes):

image

This PR is a mildly terrifying attempt at a few surgical cuts deep into the slendr internals to make ts_load() load spatial columns even for such tree sequences without breaking anything else.

With changes introduced by this PR, ts_load() and subsequent ts_nodes(ts) do what they're supposed to (note the presence of the location column as well as the fact that the data is now in the spatial sf data format):

image

The question now is, did something else break in the process? Guess we will find out!

@bodkan
Copy link
Owner Author

bodkan commented Oct 14, 2023

Oh, while I'm at this -- I had to turn off the testthat pipeline because of sudden timeout issues on GitHub Actions' part. I should try to switch this back up, to see if things have been fixed in the meantime.

@codecov
Copy link

codecov bot commented Oct 14, 2023

Codecov Report

Merging #144 (d3be498) into main (001ee58) will increase coverage by 7.25%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
+ Coverage   78.34%   85.60%   +7.25%     
==========================================
  Files           7        7              
  Lines        3117     3125       +8     
==========================================
+ Hits         2442     2675     +233     
+ Misses        675      450     -225     
Files Coverage Δ
R/tree-sequences.R 88.18% <81.81%> (+3.14%) ⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bodkan bodkan merged commit 60e0122 into main Oct 18, 2023
8 checks passed
@bodkan bodkan deleted the spatial-manual-ts branch November 28, 2023 11:58
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.

None yet

1 participant