HBandGeometry and VBandGeometry

[CiaranOMara]

This feature provides hband and vband geometries that address a need similar to that expressed in #1174.

Contributor checklist:

• I've updated the documentation to reflect these changes
• I've added an entry to NEWS.md
• I've added and/or updated the unit tests
• I've run the regression tests
• I've squash'ed or fixup'ed junk commits with git-rebase
• I've built the docs and confirmed these changes don't cause new errors

Proposed changes

• Add hband geometry that has the same width properties of hline, but a vertical span specified by ymin and ymax.
• Add vband geometry that has the same height properties of vline, but a horizontal span specified by xmin and xmax.

[codecov-io]

Codecov Report

Merging #1176 into master will decrease coverage by 0.04%.
The diff coverage is 83.67%.

@@            Coverage Diff             @@
##           master    #1176      +/-   ##
==========================================
- Coverage   93.66%   93.62%   -0.05%     
==========================================
  Files          33       35       +2     
  Lines        3156     3182      +26     
==========================================
+ Hits         2956     2979      +23     
- Misses        200      203       +3

Impacted Files	Coverage Δ
src/geometry.jl	83.33% <ø> (ø)	⬆️
src/geom/hvband.jl	100% <100%> (ø)
src/coord.jl	88.17% <100%> (ø)	⬆️
src/geom/rectbin.jl	100% <100%> (+19.23%)	⬆️
src/geom/rect.jl	77.41% <77.41%> (ø)
src/statistics.jl	95.37% <91.66%> (-0.07%)	⬇️

---

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 036dde2...cb9e44d. Read the comment docs.

[bjarthur]

this provides a nice alternate parameterization to {h,v}line, where instead of specifying the intercept and width, you specify the min and max. great to see unit tests. still needs docstrings and a gallery addition.

@miguelmorin would you find this useful.

@tlnagy @Mattriks thoughts?

thanks for contributing @CiaranOMara . and sorry for the delay getting back to you. it's been a busy summer.

[tlnagy]

Seems like a useful generalization of Geom.vline and Geom.hline. I was going to suggest that we make those a specific instance of these new geometries, but then we have to handle Geom.abline separately. I think it makes sense to organize it this way.

[mm3509]

Thanks @CiaranOMara, and yes @bjarthur I would find this very useful.

[CiaranOMara]

Thank you all for the feedback.

I'll go ahead and develop the docstrings and gallery additions for this pull request. I'll then have a go at making Geom.hline and Geom.vline specific instances of these new geometries as well as separating Geom.abline.

I'm pretty busy this winter, but I hope to make another pull-request in the next 2 weeks.

[Mattriks]

This could also perhaps be done as a RectangleBinGeometry, with Geom.band = Geom.rect + Stat.band(orientation=) . That means the color=:variable aesthetic would work, as in my #1174 (comment). Probably would need to make Geom.rect the base RectangleGeometry.

[Mattriks]

Statistics work like this (this code is bare bones, just to illustrate the grammar of graphics). Geo.rect and Geo.band have different default_statistics, and use the same render function,

module Geo
using Colors, Compose, DataFrames, Gadfly

struct RectangleGeometry <: Gadfly.GeometryElement 
    default_statistic::Gadfly.StatisticElement
    order::Int
    tag::Symbol 
end 

RectangleGeometry(default_statistic=Gadfly.Stat.identity(); order=0, tag=Gadfly.Geom.empty_tag) =
        RectangleGeometry(default_statistic, order, tag)

rect = RectangleGeometry

Gadfly.Geom.element_aesthetics(::RectangleGeometry) = [:xmin, :ymin, :xmax, :ymax, :color] 
Gadfly.Geom.default_statistic(geom::RectangleGeometry) = geom.default_statistic

band(; intercept=0.0, orientation=:vertical) =
    RectangleGeometry(BandStatistic(intercept, orientation))


function Gadfly.Geom.render(geom::RectangleGeometry, theme::Gadfly.Theme, aes::Gadfly.Aesthetics) 

    default_aes = Gadfly.Aesthetics() 
    default_aes.color = Gadfly.discretize_make_ia(RGBA{Float32}[theme.default_color]) 
    aes = Gadfly.inherit(aes, default_aes) 
    heights = aes.ymax-aes.ymin
    all(aes.ymax .== Inf) && (heights = fill(1h, length(aes.xmax)))
    
    ctx = context()
    compose!(ctx, rectangle(aes.xmin, aes.ymin, 
      aes.xmax-aes.xmin, heights, geom.tag), fill(aes.color)) 
    return compose!(context(order=geom.order), svgclass("geometry"), ctx)
end



struct BandStatistic <: Gadfly.StatisticElement
    intercept
    orientation::Symbol # :horizontal or :vertical like BarStatistic or HairStatistic
end
BandStatistic(;intercept=0.0, orientation=:vertical) = BandStatistic(intercept, orientation)

Gadfly.Stat.input_aesthetics(stat::BandStatistic) = [:xmin, :xmax, :ymin, :ymax]
Gadfly.Stat.output_aesthetics(stat::BandStatistic) = [:xmin, :xmax, :ymin, :ymax]
Gadfly.Stat.default_scales(stat::BandStatistic) = [Gadfly.Scale.x_continuous(), Gadfly.Scale.y_continuous()]

function Gadfly.Stat.apply_statistic(stat::BandStatistic,
                         scales::Dict{Symbol, Gadfly.ScaleElement},
                         coord::Gadfly.CoordinateElement,
                         aes::Gadfly.Aesthetics)
    if stat.orientation == :vertical
        aes.ymin = fill(stat.intercept, length(aes.xmin))
        aes.ymax = fill(Inf, length(aes.xmax))
    else
        aes.xmin = fill(stat.intercept, length(aes.ymin))
        aes.xmax = fill(Inf, length(aes.ymax))
    end
end

end



using RDatatsets
Dp = dataset("ggplot2","presidential")
p = plot(Dp, xmin=:Start, xmax=:End, Geo.band(), color=:Party)

There are obvious items to improve here e..g the handling of ymin and xmin.

[CiaranOMara]

Thank you @Mattriks for the advice and examples. It was not immediately apparent to me that I needed to look in statistics.jl.

I can see that in general Geom.rect can be the base geometry for RectangleBinGeometry and perhaps any other rectangular element. But, I'm unsure where to reintroduce theme modifications, for example, the subtraction of theme.bar_spacing. You noticed in my earlier attempts that I had conceived of using render in a cascade.

In my implementation of BandStatistic, I decided to allow types of Measure to pass through (see 7eb4f), but I'm not sure this is an agreeable solution. It assumes that the Measure is and will be correct for the render function.

[bjarthur]

+100 for a solution which embraces GoG and uses a statistic.

why does Theme need to be modified?

also, @Mattriks would be great to having something like your post above on how statistics work in the development section of the manual

[Mattriks]

Also great to see Geom.rect separated from the BinGeometrys (hexbin and rectbin). I also have a pca example in my Statistics notebook (section 3.0) which I would like to put into the Gadfly docs, to illustrate GoG concepts.

Theme doesn't need to be modified. The above post is simply saying that the new render function for RectangularGeometry doesn't use theme.bar_spacing as does render(geom::RectangularBinGeometry). I'm ok with that because I wouldn't want to use bar_spacing with Geom.band - but what do others think?

@CiaranOMara have you done the regression testing? That is the first test whether your new coding has any side effects!

[tlnagy]

I'm ok with that because I wouldn't want to use bar_spacing with Geom.band - but what do others think?

Agreed. I think eventually we should have subsections inside of Theme for in-graph positioning, colors, fonts, etc because it would be nice to balance specificity with verbosity of Theme.

[CiaranOMara]

@Mattriks no regression testing yet. I'll have access to my Linux box in a few days. My laptop (macOS) is affected by the _jl_libpangocairo issue and I haven't currently got a remote system setup with access.

[CiaranOMara]

I had the chance to run the regression tests to compare this development against the point at which this development forked. There are no differences between the SVGs and PNGs, except where I moved a call to Geom.rect from testscripts/rectbin.jl to testscripts/rect.jl (see a2648b1#diff-7ccc6093f9e1518e18d85bfe70640ce0L10). I'm only able to report on PNGs and SVGs because I seem to have a rendering issue with PDFs and PSs at the fork and at the tip, which I hadn't noticed before. From what I can see, this development does not introduce any regressions/side effects. But, someone else might be in a better position to check PDFs and PSs.

[CiaranOMara]

Building on an clarifying some of my of my points in relation to @tlnagy's comment.

Agreed. I think eventually we should have subsections inside of Theme for in-graph positioning, colors, fonts, etc because it would be nice to balance specificity with verbosity of Theme.

The render function for RectangularGeometry repeats all of the code pertaining to colour in RetangularBinGeometry. I assume colour behaviour should be consistent for all rendered geometries. However, I haven't followed this back to see how other geometries handle colour.

The other emergent constituent is the rendering of basic shapes; prominent in this example is the rectangle where the coordinates of its corners specify shape and position. All of the code that parses the coordinates and calls Compose's Rectangle can be the same. In the case of RetangularBinGeometry, the rectangle coordinates can be transformed as required for theme.bar_spacing, then passed on to some consistent code-block.

[bjarthur]

this looks great so far. a few suggestions:

1. delete render(geom::RectangularBinGeometry... from src/geom/rectbin.jl and have Geom.rectbin and Geom.histogram2d call the new RectangularGeometry in src/geom/rect.jl. it's a near duplicate as you have said, the only difference being the two - theme.bar_spacing.

2. workaround Geom.{rectbin,histogram2d} using theme.bar_spacing by adding another field to RectangularGeometry, which defaults to -0.05mm, and which Geom.{h,v}band overrides to be 0mm. reference this field in the two places where bar_spacing was used.

3. remove bar_spacing from Theme and add a depwarn like this.

4. we should all consider renaming Geom.rectbin to Geom.tile, since it doesn't actually bin anything. this would be more similar to ggplot's nomenclature.

[bjarthur]

maybe "pairs of intercepts" instead of "points"?

[bjarthur]

it's great to have nice examples like this that use real data sets in the documentation, but for tests i think it is better to have very simple test cases like in testscripts/rect.jl. easier to debug if necessary, will execute more quickly, and create smaller files.

[CiaranOMara]

In hindsight, I entirely agree (16af33c).

[Mattriks]

Quick comments to the points above:
1-3. Theme.bar_spacing is also also used by bar.jl, so i don't agree that bar_spacing should be removed from Theme.
4. In terms of development, I think the BinGeometries (hexbin and rectbin) should be kept separate for the moment, so they can evolve separately from the PolygonGeometries (rect and polygon).

[bjarthur]

in my mind, the BinGeoms are simply shortcuts for a Stat and a PolygonGeom.

and when you say rectbin, i think you mean histogram2d, no? rectbin doesn't actually do any binning.

[CiaranOMara]

Rects could easily be polygons too.

Gadfly.jl/src/geom/rect.jl

Lines 93 to 109 in a6fbcaf

	# TODO: determine performance hit of using polygons.
	# polys = Vector{Vector{Tuple{Measure, Measure}}}(n)
	# for i in 1:n
	# x0 = x_measure(aes.xmin[i])
	# x1 = x_measure(aes.xmax[i])
	# y0 = y_measure(aes.ymin[i])
	# y1 = y_measure(aes.ymax[i])
	# polys[i] = Tuple{Measure, Measure}[(x0, y0), (x0, y1), (x1, y1), (x1, y0)]
	# end
	#
	# return compose!(
	# context(),
	# Compose.polygon(polys),
	# fill(color),
	# stroke(nothing),
	# svgclass("geometry"),
	# svgattribute("shape-rendering", "crispEdges"))

If Geom::rect resolved to polygons, we can remove width computations where the user specifies min and max.

This may be "out there", and I certainly do not claim to know Gadfly "inside out", my intuition is that the Compose::polygon format is more useful than the Compose::rectangle. The points that define the polygon can be easily manipulated by mathematical transformations, to scale, rotate, reflect or even skew the shape. The points that define the polygon could also be projected on to other coordinate systems - admittedly, more points would need to be added on the paths between specified points to approximate curves.

[CiaranOMara]

I have split this pull request into two new pull requests so that discussions around RectangularGeometry can continue without hindering the addition of the new band geometries.
I have placed Geom.hband and Geom.vband in #1264, and RectangularGeometry in #1263.