GNOME Bugzilla – Bug 743448
ods import/export drops trend line names
Last modified: 2015-01-26 04:37:20 UTC
t6516 shows a problem with our ods import/export: we drop the name of a trend line. Trend lines have a dimension -1 containing the name. This is the name that will show up in legends. There must be a place for that someone in ods files. I just fixed the corresponding issue for xlsx. t6516-graph.pl: Check graph ods roundtrip. --- graph-tests.xml 2015-01-24 11:52:27.164064654 -0500 +++ graph-tests-new.xml 2015-01-24 11:52:27.176064653 -0500 @@ -3280,9 +3280,7 @@ <line auto-dash="0" dash="s-dot" auto-width="1" auto-color="0" color="FF:0:0:FF"/> </property> <property name="id">1</property> - <data> - <dimension id="-1" type="GnmGODataScalar">"Affine Trend"</dimension> - </data> + <data/> </GogObject> </GogObject> </GogObject> Pass
I don't think this is true for strict ODF: "There must be a place for that someone in ods files." For extended ODF one can of course use a foreign element or attribute, but in strict ODF the <chart:regression-curve> element has the following attribute: chart:style-name and no other attribute. chart:style-name gives the name of the chart style to be used, it would not make sense to include the name of the trend line in the style. Another indication that there is no place to store it is that LO does not allow a name to be set. (The names in LO represent the trend type.)
We are now truing to save and restore trendline names. Saving works fine but restoring works only for trendlines names by reference. This seems to be a larger issue though with odf_store_data since it assumes a reference. There may be other situations were we should need to handle constants or expressions.
Created attachment 295353 [details] [review] proposed patch This proposed patch should handle the trendlines names (and it does), but I get: ** (gnumeric:3450): CRITICAL **: workbook_date_conv: assertion 'wb != NULL' failed on loading.
My localc allows a name to be set. I am attaching a file with a named regression. It seems to store things with extensions, though: <style:style style:name="ch7" style:family="chart"> <style:chart-properties loext:regression-name="FooBar" loext:regression-max-degree="2" loext:regression-period="2" loext:regression-extrapolate-forward="0" loext:regression-extrapolate-backward="0" loext:regression-force-intercept="false" loext:regression-intercept-value="0" chart:regression-type="linear"/> <style:graphic-properties svg:stroke-color="#004586"/> </style:style>
Created attachment 295374 [details] File with named regression
As to why it crashes, this might shed some light: ==4665== Conditional jump or move depends on uninitialised value(s) ==4665== at 0x4EFE1C0: render_val (graph.c:86) ==4665== by 0x4EFE30C: gnm_go_data_scalar_get_str (graph.c:404) ==4665== by 0x5426188: _data_scalar_get_string (go-data.c:677) ==4665== by 0x5467426: gog_reg_curve_dataset_dim_changed (gog-reg-curve.c:347) ==4665== by 0x546C8EB: gog_dataset_set_dim (gog-data-set.c:137) ==4665== by 0x128D3216: od_series_regression (openoffice-read.c:9144) ==4665== by 0x576EB98: lookup_child (gsf-libxml.c:685) ... ==4665== Uninitialised value was created by a stack allocation ==4665== at 0x128D7B90: openoffice_file_open (openoffice-read.c:12902) My guess is that state->pos.sheet isn't valid.
Nope, state->pos.sheet is fine. But state->chart.src_sheet is uninitialized.
In localc it is apparently inside the style info rather than the regression element. That would make things somewhat more complicated... I'll try to do that later today.
In Gnumeric one can specify the name via a cell address or some arbitrary expression. Is that also possible in your localc? (I suspect it is not, there are similar cases with other properties.)
for whatever reason gsf/gsf-opendoc-utils.c contains the namespaces for ODF files, so we would need to add loext there.
In LO it's just a string. Enter "=b1" and that's what you get in the legend.
Roundtrip is now fixed for the trendline name expressions as used in Gnumeric, including interoperability with LO. Interoperability is really a bigger issue since for a long time already we also have: gnm:regression-polynomial-dims gnm:regression-affine gnm:lower-bound gnm:upper-bound f Since then localc appears to have introduced overlapping loext extensions. I note that opening the foo.ods file you attached does not show any trendline in my version of localc.
see bug #743502 for some issue with this.
Created attachment 295404 [details] Screenshot of what I see localc 4.2.7.2 I can confirm that the test now passes.
I wonder what they would say if we filed a bug suggesting that they don't make up new extensions before checking if someone else has created extension for similar features.
Created attachment 295413 [details] [review] patch re regression-max-degree This patch tries to implement loext:regression-max-degree. We already have the equivalent gnm:regression-polynomial-dims. LO also writes this attribute for linear regression (so do we), for example in foo.ods. With this patch Gnumeric dies deep in the charting engine. Now if we were going to save foo.ods from Gnumeric and change the value gnm:regression-polynomial-dims="1" to gnm:regression-polynomial-dims="2" we also end up with a segmentation fault. I will be filing a separate bug.