Fill behind scatter plot line

[GoogleCodeExporter]

For my application I have been using Core-Plot.

Yesterday I've done a small addition to the CPScatterPlot class. My version is 
now capable of rendering a margin behind a scatterplot line. I've included a 
screenshot of the concept.

What it involves is an extra optional method on the CPScatterplot datasource :
-(BOOL)showMaxMinRangeForScatterPlot:(CPScatterPlot *)plot;

When a datasource returns YES on that call. The datasource should provide 
CPScatterPlotFieldYMax & CPScatterPlotFieldYMin for that plot. The rendering of 
the CPScatterplot takes it from there.
Here's my CPScatterPlotField:
typedef enum _CPScatterPlotField {
    CPScatterPlotFieldX,                                ///< X values.
    CPScatterPlotFieldY,                                ///< Y values.
    CPScatterPlotFieldYMax,
    CPScatterPlotFieldYMin
} CPScatterPlotField;

I was wondering how I should go about getting this stuff reviewed? Fortunately 
I cloned your HG repository and have the source stored in there locally. I've 
looked over the project's site, but can't find any details on doing this sort 
of patch drop.

Again, I've got this working and in my opinion only needs a proper review by 
one of your committers.

Original issue reported on code.google.com by jeroen.l...@gmail.com on 12 Jan 2011 at 9:42

Attachments:

• plot-range-fill.png

[GoogleCodeExporter]

This looks like a nice addition. I recommend a few changes to the API:

- Maybe call the enum fields CPScatterPlotFieldBackgroundAreaMaximumY, etc, 
just to make clear what it is.
- Don't use a data source method for turning this on and off. Just make a BOOL 
property called 'drawsBackgroundArea' on CPScatterPlot
- Add a property called 'backgroundAreaFill' to CPScatterPlot, and use that to 
fill the area.

If you make those changes, email me or post and we'll get the patch sorted.

Thanks!

Original comment by drewmcco...@mac.com on 12 Jan 2011 at 10:01

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Actually, I've changed my mind. I think this functionality would fit much 
better in CPRangePlot. It shows a range, and that's what CPRangePlot does, just 
in a different style. Best would probably be to add a enum for CPRangePlotType, 
with options for CPRangePlotTypeAreaFill, and CPRangePlotTypeBar. You would 
then set the property 'rangePlotType' on the CPRangePlot class.

This would have the advantage that the area fill would work for both x and y.

Can you take a look at that? I suspect it is not much different to doing it in 
scatter plot, but has a bunch of advantages in terms of flexibilty.

Original comment by drewmcco...@mac.com on 12 Jan 2011 at 10:08

[GoogleCodeExporter]

Actually, don't have the enum in CPRangePlot. Just have boolean properties for 
showsAreaFill and showsBars. By default, only bars should be on, but you can 
turn one or both on.

Original comment by drewmcco...@mac.com on 12 Jan 2011 at 10:10

[GoogleCodeExporter]

Sounds simple enough.

I'll look into it. Seems like RangePlot is pretty much the same mechanism as I 
am currently using for my change.

Original comment by jeroen.l...@gmail.com on 12 Jan 2011 at 10:35

[GoogleCodeExporter]

I've looked a bit at the RangePlot class. It doesn't support a line like the 
scatter plot currently has.

What do you think is better? Have two separate plots, one showing the range 
with a range plot and another drawing a scatter plot, or have the range and 
line be drawn by one plot.

Original comment by jeroen.l...@gmail.com on 12 Jan 2011 at 10:55

[GoogleCodeExporter]

I actually prefer having two plots. I think that is more flexible. For a start, 
maybe you don't want a line. If you put it all in scatter plot, the class 
becomes a kitchen sink. You end up having to do hacks if you don't want a 
center line, such as setting the line style to nil, and returning dummy data. 

I like the orthogonality of having two separate plot classes that can be 
combined in different ways. Maybe you even want the range on top of the scatter 
plot. Stuff like that.

Original comment by drewmcco...@mac.com on 12 Jan 2011 at 11:14

[GoogleCodeExporter]

I get your point, but wouldn't it be best then to actually expand the 
CPTradingRangePlotStyle by adding an enumeration?

typedef enum _CPTradingRangePlotStyle {
    CPTradingRangePlotStyleOHLC,        ///< OHLC
    CPTradingRangePlotStyleCandleStick, ///< Candle
    CPTradingRangePlotStyleFill

} CPTradingRangePlotStyle;

I wouldn't want to create a kitchen sink there either. ;) A framework user who 
would like a a fill behind their candlestick or OHLC plot would just need to 
supply the same datasource to an extra plot with a different style. Unless 
there are severe performance penalties for doing this I would imagine this 
being the most flexible, maintainable and orthogonal.

Original comment by jeroen.l...@gmail.com on 12 Jan 2011 at 11:58

[GoogleCodeExporter]

No, the trading range plot is a whole different beast. It is a specialized 
financial plot, and takes 4 different numbers: open, high, low, close. 

CPRangePlot is the appropriate place, because that is exactly what you are 
showing: a range. You can show it in two ways: with an area fill, or with a 
bar. But in both cases, it is showing a range.

Original comment by drewmcco...@mac.com on 12 Jan 2011 at 12:12

[GoogleCodeExporter]

Ah wait, I need to update my tree. :) I'll get to this once I've got some time 
to work on it. I'm currently looking at 964:0c2f5aef86b1 tip.

Original comment by jeroen.l...@gmail.com on 12 Jan 2011 at 12:30

[GoogleCodeExporter]

Well here's a patch for it. Not sure how to work with this HG stuff in relation 
to the core-plot repository. This hg export seems to show the changes I made.

Original comment by jeroen.l...@gmail.com on 13 Jan 2011 at 1:27

Attachments:

• patchCPRangePlotFill.txt

[GoogleCodeExporter]

I've added to the example as well. Just click on the NSRange example window to 
let the graph redraw itself with a fill.

Original comment by jeroen.l...@gmail.com on 13 Jan 2011 at 1:28

[GoogleCodeExporter]

This patch has now been incorporated in the main branch of Core Plot. The API 
was modified a little, and the ability to set any CPFill was added, but most of 
the work is as Jeroen did it.

Original comment by drewmcco...@mac.com on 13 Jan 2011 at 12:12

• Changed state: Fixed