Added more chart supports.#144
Added more chart supports.#144sandeeptiwari32 wants to merge 6 commits intoapache:trunkfrom sandeeptiwari32:trunk
Conversation
fixed bug while creating chart with bar and line series.
|
Can one of the admins verify this patch? |
|
|
||
| // the line chart | ||
| // the line chart on secondary axis | ||
| XDDFLineChartData lines = (XDDFLineChartData) chart.createData(ChartTypes.LINE, lineCategories, |
There was a problem hiding this comment.
create line series with secondary axis
|
|
||
|
|
||
| XDDFLineChartData.Series series2 = (XDDFLineChartData.Series) lines.addSeries(xs, ys2); | ||
| series2.updateIdXVal(1); |
There was a problem hiding this comment.
update axis value and order value of series.
| package org.apache.poi.xddf.usermodel.chart; | ||
|
|
||
| public enum ChartTypes { | ||
| AREA, |
There was a problem hiding this comment.
enum for new chart types
| @@ -0,0 +1,165 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for area 3d chart
| @@ -0,0 +1,168 @@ | |||
| /* ==================================================================== | |||
| @@ -0,0 +1,201 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for bar 3d chart
| protected CTNumDataSource getNumDS() { | ||
| return series.getVal(); | ||
| } | ||
|
|
There was a problem hiding this comment.
overrided method to update series id and order id
There was a problem hiding this comment.
I can see these methods, but your comment does not help to understand what problem they were supposed to solve in the first place.
I first of all question their public visibility. Since I don't think it is good practice to rely on the developer using the library to know when to use and what to expect from them...
There was a problem hiding this comment.
so we created update methods, because in a chart, we need to provide unique value to each series, now while creating combo chart, i.e. line series and bar series in that case we assigning same values to each series due to that it was corrupting the office file. Now either we can create a new constructor with new parameter i.e. index or can provide an update method to re-assign axis id and order id. so i chose update method.
There was a problem hiding this comment.
I finally understood what you were trying to achieve! I had a similar code in a PR that I left aside because of some more changes required after a comment about API requiring changes.
https://github.com/apache/poi/pull/139/files#diff-17a515ee7210ab25ee2bcedbf5e49a87R116
There was a problem hiding this comment.
I also tried to get all series list in chart data, but as there is no relationship between different series types, so we are left with only series list of particular chart data.
| * | ||
| * @return this method will add 3D view | ||
| */ | ||
| public XDDFView3D getOrAddView3D() |
There was a problem hiding this comment.
method to add 3d view into chart
| Map<Long, XDDFChartAxis> categories = null; | ||
| Map<Long, XDDFValueAxis> mapValues = null; | ||
|
|
||
| if(ChartTypes.PIE != type && ChartTypes.PIE3D != type) |
There was a problem hiding this comment.
handled null pointer exception in case of pie chart.
| } | ||
|
|
||
| final CTPlotArea plotArea = getCTPlotArea(); | ||
| switch (type) { |
There was a problem hiding this comment.
added case to create instance for new added chart
|
|
||
| protected abstract CTNumDataSource getNumDS(); | ||
|
|
||
| protected abstract void updateIdXVal(long val); |
There was a problem hiding this comment.
abstract methods to update series id and order id
| @@ -0,0 +1,233 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for Line 3d chart
| } | ||
|
|
||
| public void setGrouping(Grouping grouping) { | ||
| if (chart.getGrouping()!=null) { |
There was a problem hiding this comment.
handled case in case of grouping was not created in chart
| @@ -0,0 +1,157 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for pie 3d chart
| @@ -0,0 +1,168 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for surface 3d chart
| @@ -0,0 +1,167 @@ | |||
| /* ==================================================================== | |||
There was a problem hiding this comment.
support for surface chart
| @@ -0,0 +1,106 @@ | |||
| /* | |||
There was a problem hiding this comment.
created class to use 3d view
Alain-Bearez
left a comment
There was a problem hiding this comment.
After formatting details are fixed, we will start discussing some more in depth design decisions.
|
|
||
| // the bar chart | ||
| XDDFBarChartData bar = (XDDFBarChartData) chart.createData(ChartTypes.BAR, lineCategories, rightValues); | ||
| XDDFBarChartData bar = (XDDFBarChartData) chart.createData(ChartTypes.BAR, barCategories, leftValues); |
|
|
||
| XDDFLineChartData.Series series2 = (XDDFLineChartData.Series) lines.addSeries(xs, ys2); | ||
| series2.updateIdXVal(1); | ||
| series2.updateOrderVal(1); |
There was a problem hiding this comment.
Your updateXXXVal() methods are disguised setters. I am not quite sure this is the best way to achieve your goal (which I still does not understand at this point).
There was a problem hiding this comment.
so we created update methods, because in a chart, we need to provide unique value to each series, now while creating combo chart, i.e. line series and bar series in that case we assigning same values to each series due to that it was corrupting the office file. Now either we can create a new constructor with new parameter i.e. index or can provide an update method to re-assign axis id and order id. so i chose update method.
| SCATTER | ||
| SCATTER, | ||
| SURFACE, | ||
| SURFACE3D |
There was a problem hiding this comment.
Seven new chart types at once!!!
Very impressive, indeed!
| protected CTNumDataSource getNumDS() { | ||
| return series.getVal(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I can see these methods, but your comment does not help to understand what problem they were supposed to solve in the first place.
I first of all question their public visibility. Since I don't think it is good practice to rely on the developer using the library to know when to use and what to expect from them...
| public XDDFView3D getOrAddView3D() | ||
| { | ||
| CTView3D view3D; | ||
| if(chart.isSetView3D()) { |
There was a problem hiding this comment.
missing whitespace between if and (
|
|
||
| @Override | ||
| public void setVaryColors(boolean varyColors) { | ||
|
|
|
|
||
| @Override | ||
| public void setShowLeaderLines(boolean showLeaderLines) { | ||
|
|
| view3D.getRotY().setVal(val); | ||
| } | ||
| else | ||
| view3D.addNewRotY().setVal(val); |
There was a problem hiding this comment.
The three lines above don't follow formatting rules for the project...
And the pattern is (wrongly) repeated until the end of this class!
| } | ||
|
|
||
| public int getRotXVal() { | ||
| return view3D.getRotX().getVal(); |
There was a problem hiding this comment.
Despite being internally represented as an int value, the rotation represents an angle which developers using this library will expect to be expressed in degrees instead of the scaled range allowed by the specs.
We will have to provide a converting mechanism. I am sorry I got it wrong in the first place on the other properties representing angles...
There was a problem hiding this comment.
yes we need to write a utility method for conversion.
There was a problem hiding this comment.
i am writing that utility method. so if it's completed early so i will check in that code with this pull request.
There was a problem hiding this comment.
While reading the specs, this is one of the few places where the angles are given as integer values, without scaling by 60,000 to get more precision.
| return view3D.getHPercent().getVal(); | ||
| } | ||
|
|
||
| public void setHPercentVal(int val) { |
There was a problem hiding this comment.
As matter of facts, all the methods names have to be adapted to the conventions used otherwise in the project:
- no
Valsuffix - no unpronounceable name such as
RAngAx - no shortcut as
RotX,RotYorHPercentbutRotationX,RotationYandHeightPercent
sandeeptiwari32
left a comment
There was a problem hiding this comment.
updated suggested changes.
|
applied as r1859590 |
closes #139 closes #144 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1859676 13f79535-47bb-0310-9956-ffa450edef68
closes apache#139 closes apache#144 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1859676 13f79535-47bb-0310-9956-ffa450edef68
Added more chart supports.
fixed bug while creating chart with bar and line series.