bugGNU Octave - Bugs: bug #43331, io-2.2.4 package - function...

 
 

bug #43331: io-2.2.4 package - function xlsread -> function __OCT_xlsx2oct__ in inst/private/__OCT_xlsx2oct__.m

Submitter:  None
Submitted:  Tue 30 Sep 2014 02:46:51 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Marco Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * other
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 05 Oct 2014 09:59:12 AM UTC, comment #9: 

@Markus: While looking into bug #43354, I saw that I already took care of updating the rels/worksheet.xml.rels worksheet bookkeeping in __OCT_oct2xlsx_.m for writing new sheets (the _rels stuff did ring a bell here; turns out it was in the write stuff).
So I wonder why it never made it properly into the read stuff.
Anyway, it seems it is fixed now.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 04 Oct 2014 06:51:40 PM UTC, comment #8: 

@Markus:
Applied your patch after a few cosmetic mods + one more check for rawsheet fid. Thank you very much for the fix.

I noted from your patch that some CRLF from my side creeped in => must be bug #43334.

I'm afraid we might have to do something about writing the _rels stuff as well, once that file is in the xlsx archive. Something for another weekend? :-(


Philip Nienhuis <philipnienhuis>
Group Member
Thu 02 Oct 2014 07:33:56 PM UTC, comment #7: 

I've reworked my patch (ignore the first one). It reads now workbook.rels.xml into xls.sheets.rels - when it's available. - and used it - when sheet%d isn't find in the first try. Furhtermore, I've absolutly no clue about the priority order to open sheet%d by workbook.xml (sheetid or r:id) or by _rels/workbook.rels.xml ...



(file #32216)

Markus Bergholz <markuman>
Thu 02 Oct 2014 04:35:53 PM UTC, comment #6: 

Thanks for the files.
OOXML is complicated almost beyond belief, from what I understand we've got to open yet another xml file to hunt down pointers where in other file formats just one pointer will do. Oh well.

I'll look over the patch (thx Markus) + push it and see if your suggestions in comment #5 can be incorporated on top of it.

BTW do you have a direct pointer (URL) to the spec pertaining to the worksheet bookkeeping?

& It would be nice if you have more test cases....

Philip Nienhuis <philipnienhuis>
Group Member
Thu 02 Oct 2014 11:15:06 AM UTC, comment #5: 

Dear Philip&Markus,

the quick fix works (patch #32206) for the time being. Thanks!

BUT: I would recommend extracting the entire Target="..." attribute from the worksheet.xml.rels file's <Relationship .../> tag. In the described special case of the bug43331-addnumber.xlsx file, the r:id="+b+...-b-" happens to be the same as the sheet+b+...-b-.xml number, but this is not required.

What IS required from the ISO specs is that the r:Id attribute value of the workbook.xml <sheet .../> tag has a corresponding Id attribute value in the workbook.xml.rels file's <Relationship .../> tag.

So, in my view, the bug could be fixed by having xls.sheets.rid(id) return "rId1" (in the case of bug43331-addnumber.xlsx) rather than only "1". Then, this "rId1" is searched for in workbook.xml.rels as the corresponding Id attribute value of the <Relationship .../> tag, and the entire attribute value of Target is fed into the function call for the worksheet = fopen(sprintf('%s', xls.workbook, +b+wsh-b-)), where +b+wsh-b- would have to be replaced by the attribute value of Target.

Hope that helps.

Best,
Marco

Anonymous
Wed 01 Oct 2014 05:35:08 PM UTC, comment #4: 

I've attached a bugfix patch and the example .xlsx file.

(file #32206, file #32207)

Markus Bergholz <markuman>
Tue 30 Sep 2014 07:21:46 PM UTC, comment #3: 

@Markus: I did already fix a few issues concerning rIds for -IIRC- io-2.2.2. I'm not surprised that the code in question (largely mine) still might need improvement.

@OP: again, please provide an example worksheet that portrays the bug. You didn't actually upload the "Sample.xlsx" file

And, as you seem to have good understanding of the OOXML specs, could you provide a changeset? or maybe even just a diff -u patch? (against io-2.2.4)

Philip Nienhuis <philipnienhuis>
Group Member
Tue 30 Sep 2014 05:49:48 PM UTC, comment #2: 

I guess it's easy to fix. But we need an example file which reproduce this behavior.

Neither I have read the ISO/IEC 29500 nor I've bought it.

Markus Bergholz <markuman>
Tue 30 Sep 2014 04:08:57 PM UTC, comment #1: 

Thanks for your bug report.

The OCT xlsx stuff does need some polishing, indeed.
I'll have a look at your suggestions later this week.

In the mean time, could you please upload to this bug report your worksheet, or a stripped down copy of it, that shows the error?

Philip Nienhuis <philipnienhuis>
Group Member
Tue 30 Sep 2014 02:46:51 PM UTC, original submission:  

Dear all,

when searching for a sheet by name <wsh>, the above function searches for the numerical attribute “sheetId” of the <sheet /> tag from the workbook.xml file. Then, in the rawsheet assignment, it searches for the file ‘%s/xl/worksheets/sheet%d.xml”, where %d is the number retrieved from sheetId.

For some reason, my sample file has a sheetId (%d) which does not correspond to a “%s/xl/worksheets/sheet%d.xml” file. But the file _rels/worksheets.rels.xml does contain an entry <Relationship Id=”rId1” Type=”…” Target=”sheet3.xml” /> entry. And the Id “rId1” is correctly set in the worksheets.xml file.

So, I believe that the search for the file should be based on the r:Id=”rId1” attribute via the _rels/worksheet.rels.xml file rather than the number retrieved from sheetId=”…” (see also ISO/IEC 29500-1:2012(E) in chapter 12.2 “Package Structure”).

Here are some bits of the sample xml files included in the xlsx file on which the error is produced:



file "xl\workbook.xml"
<sheet r:id="rId1" sheetId="15" name="XXX"/>

file "xl\_rels\workbook.xml.rels"
<Relationship Target="worksheets/sheet1.xml" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/worksheet" Id="rId1"/>


The file "xl\worksheets\sheet1.xml" exists while the file "xl\worksheets\sheet15.xml" does not.

Please see the attached Sample.xlsx file and run the following command on it (with "name_of_sample_file" according to your path):


xlsread( name_of_sample_file , 'XXX' , 'A1:B2' ) ;


Thanks in advance!

Best Regards,
Marco

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #32216:  bugfix-43331-2.patch added by markuman (6KiB - text/x-patch)
file #32206:  bugfix-43331.patch added by markuman (3KiB - text/x-patch)
file #32207:  bug43331-addnumber.xlsx added by markuman (8KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by markuman (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-04 philipnienhuis StatusNone Fixed
        Open/ClosedOpen Closed
        Release3.8.1 other
        Operating SystemMicrosoft Windows Any
    2014-10-02 markuman Attached File- Added bugfix-43331-2.patch, #32216
    2014-10-01 markuman Attached File- Added bugfix-43331.patch, #32206
        Attached File- Added bug43331-addnumber.xlsx, #32207

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code