Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Looking good...

Hi @leehart, couple of comments...

One very small request, would it be possible to add just a little more vertical padding around the text labels? Particularly the negative strand gene labels are touching the gene rectangles, would be good to have a little padding.

The other request is that ultimately this will be needed as part of functions like plot_h12_gwss() where there is a genes track plotted below some other data track. So basically, could you add a gene_labels parameter to all functions like this, i.e., where plot_genes() is called internally?

@alimanfoo How does this padding look to you?

Ideally I'd like a more robust mechanism for accommodating these labels, because this is currently tailored to the default height of the plot, i.e. 120, so specifying a different height, e.g. height=240, results in a skewed layout, e.g.

@alimanfoo How does this padding look to you?

Looks good!

Ideally I'd like a more robust mechanism for accommodating these labels, because this is currently tailored to the default height of the plot, i.e. 120, so specifying a different height, e.g. height=240, results in a skewed layout, e.g.

Thanks @leehart, I think it's probably ok for now, generally unlikely to need to vary the height of this track by much.

Hi @leehart, could we rename the labels param to gene_labelset? That way when the parameter appears in functions like plot_h12_gwss() it will be more obvious that the parameter will apply to the genes track.

Codecov Report

Attention: Patch coverage is 12.50000% with 28 lines in your changes missing coverage. Please review.

Project coverage is 94.57%. Comparing base (af6bac1) to head (cc1ce67).
Report is 79 commits behind head on master.

@@            Coverage Diff             @@
##           master     #653      +/-   ##
==========================================
- Coverage   95.19%   94.57%   -0.62%     
==========================================
  Files          40       40              
  Lines        4144     4187      +43     
==========================================
+ Hits         3945     3960      +15     
- Misses        199      227      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Looks like codecov/patch wants some test coverage for the new gene_labels param. Will do.

Looks like codecov/patch wants some test coverage for the new gene_labels param. Will do.

Cool, thanks @leehart. Happy to be merged once codecov/patch passes.