Skip to content

Conversation

@hyanwong
Copy link
Member

@hyanwong hyanwong commented Feb 22, 2021

And correct some alignment bugs in SVG placement of trees in a tree sequence, where we accidentally padded the top of the trees in the tree sequence, rather than just the bottom.

This will result in nicer diagrams for the tutorial, and is an obvious addition. The only question is whether we want a label by default or not. It's less of a change, and less language-specific if we simply have no X label by default, so I just left it that way for the time being.

Visual representations to check this is what we want are in the tests/data/svg directory.

PR Checklist:

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

@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #1213 (00767e1) into main (21da9b0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1213   +/-   ##
=======================================
  Coverage   93.72%   93.72%           
=======================================
  Files          26       26           
  Lines       21516    21518    +2     
  Branches      905      906    +1     
=======================================
+ Hits        20166    20168    +2     
  Misses       1312     1312           
  Partials       38       38           
Flag Coverage Δ
c-tests 92.50% <ø> (ø)
lwt-tests 92.97% <ø> (ø)
python-c-tests 94.91% <100.00%> (+<0.01%) ⬆️
python-tests 98.64% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
python/tskit/trees.py 97.42% <ø> (ø)
python/tskit/drawing.py 99.10% <100.00%> (+<0.01%) ⬆️

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 21da9b0...00767e1. Read the comment docs.

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.

Looks good. I can't see any x-label in the example when I view it on github (and search this page for "genomic position" doesn't hit anything in the xml, so I'm not sure it's actually worked?

And correct some alignment bugs in SVG placement of trees in a tree sequence
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, very useful

@mergify mergify bot merged commit 409361c into tskit-dev:main Feb 23, 2021
@hyanwong hyanwong deleted the svg-x-label branch February 26, 2021 16:57
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