Clean up axis expansion

[hadley]

• Currently it's difficult to follow the code that computes axis expansion. The code (and default values) are spread out over both coordinate systems and scales.
• We need to distinguish between setting the limits of a coordinate system exactly and approximately (e.g. expanded and not expanded). It's not exactly clear what the correct behaviour. It looks like the old wise parameter used to control expansion, but was removed, with the claim that expansion was now the default behaviour.
• Given that the behaviour has clearly changed over the last few years, and I don't remember any complaints about it, it's probably not a heavily used feature so we can change it to do the "right" thing.
• I would prefer to keep the expand parameter in one place (the scale), but I think there is some interaction with the coord, because the default expansion varies based on the coordinate system. Maybe the coordinate system could have a boolean expand flag, with the details controlled in the scale.

cc @wch, @lionel-

[hadley]

It would be a hell of a lot easier to understand this code if I'd documented anything 😢. Here's what I've figured out so far:

• The job of train is to take the x and y scales (currently in a list, but should be individual arguments) and return a "scale details" object.
• A "scales detail" object is a named list which contains things like x.range, y.major, .... One scales detail object is computed for each panel and becomes the scales argument that's passed into each draw method. It's the job of the draw method to call coordinates$transform(data, scales) (at least once for each panel)

The code would be a lot easier to follow if the argument names referring to the "scales details" object were consistent - sometimes it's called scales, and sometimes scale_details. (And in $train(), the scales_details argument is actually a list of scales)

[lionel-]

Maybe the coordinate system could have a boolean expand flag, with the details controlled in the scale.

IIUC that is basically how the old wise argument worked.

I think I would prefer full feature parity between coords and scales for the sake of consistency (i.e., using a multiplicative and additive expansion), but the advantage of a boolean is a simpler interface. Plus the boolean would work in all use cases because it would ensure a consistent default expansion while still allowing fine-grained control of limits when it's switched off.

Edit: I was confused

[BrianDiggs]

Also think about how transformations fit into this (since there is data space, transformed data space, and coordinate space); #980 relates to these problems.

[hadley]

@lionel- the disadvantage of full feature parity is then its confusing where you should set things. I'd definitely prefer a boolean here.

@BrianDiggs that's beyond the scope of this issue, unfortunately

[bryanhanson]

Am I the only one who thinks the first line should be a fortune? I leave it to Hadley to self-nominate. ;-)

On Jul 27, 2015, at 4:37 PM, Hadley Wickham notifications@github.com wrote:

It would be a hell of a lot easier to understand this code if I'd documented anything . Here's what I've figured out so far:

The job of train is to take the x and y scales (currently in a list, but should be individual arguments) and return a "scale details" object.

A "scales detail" object is a named list which contains things like x.range, y.major, .... One scales detail object is computed for each panel and becomes the scales argument that's passed into each draw method. It's the job of the draw method to call coordinates$transform(data, scales) (at least once for each panel)

The code would be a lot easier to follow if the argument names referring to the "scales details" object were consistent - sometimes it's called scales, and sometimes scale_details. (And in $train(), the scales_details argument is actually a list of scales)

—
Reply to this email directly or view it on GitHub #1207 (comment).

[hadley]

This now looks good to me. Please let me know what you think.

[lionel-]

it works brilliantly. Thanks!