After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 743448 - ods import/export drops trend line names
ods import/export drops trend line names
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: import/export OOo / OASIS
git master
Other All
: Normal normal
: ---
Assigned To: Andreas J. Guelzow
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2015-01-24 16:55 UTC by Morten Welinder
Modified: 2015-01-26 04:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.30 KB, patch)
2015-01-25 04:57 UTC, Andreas J. Guelzow
none Details | Review
File with named regression (16.40 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-01-25 13:57 UTC, Morten Welinder
  Details
Screenshot of what I see (77.58 KB, image/png)
2015-01-25 22:41 UTC, Morten Welinder
  Details
patch re regression-max-degree (1.80 KB, patch)
2015-01-26 04:37 UTC, Andreas J. Guelzow
none Details | Review

Description Morten Welinder 2015-01-24 16:55:53 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">&quot;Affine Trend&quot;</dimension>
-                    </data>
+                    <data/>
                   </GogObject>
                 </GogObject>
               </GogObject>
Pass
Comment 1 Andreas J. Guelzow 2015-01-24 20:32:30 UTC
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.)
Comment 2 Andreas J. Guelzow 2015-01-24 23:13:15 UTC
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.
Comment 3 Andreas J. Guelzow 2015-01-25 04:57:14 UTC
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.
Comment 4 Morten Welinder 2015-01-25 13:56:30 UTC
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>
Comment 5 Morten Welinder 2015-01-25 13:57:12 UTC
Created attachment 295374 [details]
File with named regression
Comment 6 Morten Welinder 2015-01-25 14:31:35 UTC
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.
Comment 7 Morten Welinder 2015-01-25 14:41:02 UTC
Nope, state->pos.sheet is fine.
But state->chart.src_sheet is uninitialized.
Comment 8 Andreas J. Guelzow 2015-01-25 17:15:38 UTC
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.
Comment 9 Andreas J. Guelzow 2015-01-25 17:50:45 UTC
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.)
Comment 10 Andreas J. Guelzow 2015-01-25 18:12:48 UTC
for whatever reason gsf/gsf-opendoc-utils.c contains the namespaces for ODF files, so we would need to add loext there.
Comment 11 Morten Welinder 2015-01-25 21:21:07 UTC
In LO it's just a string.  Enter "=b1" and that's what you get in the legend.
Comment 12 Andreas J. Guelzow 2015-01-25 21:39:47 UTC
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.
Comment 13 Andreas J. Guelzow 2015-01-25 22:01:06 UTC
see bug #743502 for some issue with this.
Comment 14 Morten Welinder 2015-01-25 22:41:52 UTC
Created attachment 295404 [details]
Screenshot of what I see

localc 4.2.7.2

I can confirm that the test now passes.
Comment 15 Morten Welinder 2015-01-25 22:43:44 UTC
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.
Comment 16 Andreas J. Guelzow 2015-01-26 04:37:20 UTC
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.