graphite-project/graphite-web — pull request #344 by bitprophet (2013-06-06 04:00:01+00:00)
Title: Fix yMaxValue when areaMode=stacked

## Problem

The yAxis' upper limit may be miscalculated (too large/tall) when timeseries consolidation occurs & that consolidation paves over significant outliers in the underlying data.

This only occurs when viewing the graph with `areaMode=stacked`; all other `areaMode`s are fine.

## Steps to replicate

* Obtain a time series segment that has more points than pixels (forcing consolidation) and which has some significant outliers that would normally manifest as graph spikes.
* Graph it with `areaMode=none` or whatnot; observe that the Y axis reflects the consolidated data's max values. [Example](http://f.cl.ly/items/0c3Y3U1w3R3p352M183q/Image%202013.06.05%208%3A42%3A30%20PM.png)
* Switch to `areaMode=stacked`; observe that while the actual "profile" of the graphed data doesn't change, the Y axis grows a bunch, creating empty space on the upper ~half of the graph (depending on how big your outliers were). [Example](http://f.cl.ly/items/0w1G2z1d361z1W1J183R/Image%202013.06.05%208%3A42%3A09%20PM.png)

## Investigation

So, as it turns out, the code in `setupYAxis` that determines that maximum Y axis value does two subtly different things to the `TimeSeries` objects involved:

* When `areaMode` is not `stacked`, it simply obtains the maximum value across all series being graphed, then gets the maximum of that, to get the graph's global maximum. Easy peasy.
* When `areaMode=stacked`, it appears to reinvents `zip` (I can't tell why) and uses a combination of `len(series)` and `series[i]` to do so.
* When no consolidation has occurred, the result is the same for both `areaMode`s. Hooray!
* When consolidation occurs, we hit a snag:
* `TimeSeries` inherits from `list`, and overrides `__iter__` to handle consolidation on the fly.
* But it does not redefine `len` or `__getitem__`!
* Therefore, `len(series)` and `series[i]` always operate on the "real" underlying list data -- but iteration operates on the consolidated data -- leading to the mismatch between the graphed data and the calculated Y-axis maximum.

## Solution A: correct but probably bug-prone

Implement `len` and `__getitem__` "correctly" in `TimeSeries` such that they reflect any consolidation. Unfortunately, because consolidation is done on the fly via a generator, this would require that strategy to change to e.g. caching the consolidated copy of the data whenever `consolidate()` is called.

If that were done, all other list-like API calls could be implemented to focus on the consolidated copy and everything's consistent.

However, I suspect that this could have wide-reaching effects and might well surface bugs elsewhere that rely (right or wrong) on the current behavior.

## Solution B: the safer but still awesome workaround

Just use `itertools.zip` to replace the code that calls `len` and `__getitem__`. This function behaves exactly like the code it replaces (AFAICT) but also uses the iterator API to access the `TimeSeries` objects, thus ensuring it works right with consolidated data.

I have implemented Solution B in this pull request, but am open to exploring Solution A if smart people think it's worthwhile. Willing to bet other bugs exist hinging on this same core problem.