zdavatz / spreadsheet

The Ruby Spreadsheet by ywesee GmbH
http://spreadsheet.ch
GNU General Public License v3.0
1.13k stars 240 forks source link

Delete row not working on some files #139

Closed runephilosof closed 7 years ago

runephilosof commented 9 years ago

Using the file in #137 (https://github.com/zdavatz/spreadsheet/raw/f0ed6b347813d96fb4c4aeaca7cac5cc87734be1/test/data/test_delete_row.xls) this does not work:

      path = File.join @data, 'test_delete_row.xls'
      book = Spreadsheet.open path
      sheet1 = book.worksheet 0
      assert_equal "row1", sheet1[0,1]
      assert_equal "row2", sheet1[1,1]
      pre_count = sheet1.row_count
      sheet1.delete_row 0
      assert_equal pre_count - 1, sheet1.row_count
      assert_equal "row2", sheet1[0,1]
      assert_equal "", sheet1[1,1]
zdavatz commented 9 years ago

what Ruby version are you using?

zdavatz commented 9 years ago

I get this error with Ruby-1.9.3

/tmp> ruby test.rb 
test.rb:3:in `join': can't convert nil into String (TypeError)
        from test.rb:3:in `<main>'

Do you get the same error?

zdavatz commented 9 years ago

Same error with Ruby 2.2.0 on Linux. I do not get passed the line:

path = File.join @data, 'test_delete_row.xls'
runephilosof commented 9 years ago

It should probably be

path = 'test_delete_row.xls'

Depending on where you put the file of course.

Regard version for the rest I am running 2.1.6. But it fails in 1.8.7, 1.9.2, 1.9.3, 2.1.1 and 2.0.0 according to this travis run https://travis-ci.org/zdavatz/spreadsheet/builds/67336264

zdavatz commented 9 years ago

The test script now goes:

require "spreadsheet"

path = 'test_delete_row.xls'
book = Spreadsheet.open path
sheet1 = book.worksheet 0
assert_equal "row1", sheet1[0,1]
assert_equal "row2", sheet1[1,1]
pre_count = sheet1.row_count
sheet1.delete_row 0
assert_equal pre_count - 1, sheet1.row_count
assert_equal "row2", sheet1[0,1]
assert_equal "", sheet1[1,1]

and results in

/tmp> ruby test.rb 
test.rb:6:in `<main>': undefined method `assert_equal' for main:Object (NoMethodError)

So your test script still seems not to be working.

I am using Ruby 2.2.0 on Linux with Kernel 4.1.

runephilosof commented 9 years ago

Well the test script was a snippet for test/integration.rb, so the assert_equal would work there (#137). We could replace it with:

require "spreadsheet"

path = 'test_delete_row.xls'
book = Spreadsheet.open path
sheet1 = book.worksheet 0
assert_equal "row1", sheet1[0,1]
assert_equal "row2", sheet1[1,1]
pre_count = sheet1.row_count
sheet1.delete_row 0
if pre_count - 1 == sheet1.row_count &&
    "row2" == sheet1[0,1] && 
    "" == sheet1[1,1]
  p "success"
else
  p "failure"
end
zdavatz commented 9 years ago

Now Ruby 2.2.2 of above test.rb is returning

test.rb:6:in `<main>': undefined method `assert_equal' for main:Object (NoMethodError)

on my Linux machine.

runephilosof commented 9 years ago

Lets just remove those two rows.

require "spreadsheet"

path = 'test_delete_row.xls'
book = Spreadsheet.open path
sheet1 = book.worksheet 0
pre_count = sheet1.row_count
sheet1.delete_row 0
if pre_count - 1 == sheet1.row_count &&
    "row2" == sheet1[0,1] && 
    "" == sheet1[1,1]
  p "success"
else
  p "failure"
end

Now on my ruby 2.1.6 it outputs "failure"

zdavatz commented 9 years ago

Ok, now Ruby 2.2.2 gives me:

/tmp> ruby test.rb 
"failure"
/tmp> 

so now what do you suggest?

wvengen commented 8 years ago

Trying several xls files created by Libreoffice 4.4.2.2, I can't seem to delete a row:

sheet = Spreadsheet.open('test.xls').worksheet(0)
puts "Before: #{sheet.row_count}"
sheet.delete_row(0)
puts "After: #{sheet.row_count}"

This consistently shows equal counts before and after.

Spreadsheet 1.0.7 with Ruby 2.1.5. Also seen with spreadsheet 1.0.0.

zdavatz commented 8 years ago

please link the files here so I can test them.

zdavatz commented 8 years ago

I took an empty file and added "test" in the first cell, created by LibreOffice 5.0.1.2, spreadsheet 0.9.4 and ran your script with Ruby 1.9.3-p429 and then I got

Before: 1
After: 1

I get the same result with spreadsheet-1.0.7 - the test file ist here: http://1drv.ms/1LvJna1

zdavatz commented 8 years ago

Have you seen this? You will also have to write to a new file when you edit something in the file. https://gist.github.com/wimnorder/a2eba22a9c815b32c8d4

wvengen commented 8 years ago

Here's a spreadsheet that shows 3 rows before and after: test.xls

This is not about writing to a file, this is about modifying it in-memory. Or does the change only happen when the file is written?

zdavatz commented 8 years ago

I guess so.

ani1 commented 8 years ago

@zdavatz I am also getting same error. Is there any solution?

andrecnogueira commented 8 years ago

Hey guys, Is there any update on this issue? I can't delete rows..

ruby 2.2.4p230 (2015-12-16 revision 53155) [x64-mingw32]

require 'rubygems' require 'spreadsheet'

Spreadsheet.client_encoding = 'UTF-8'

book = Spreadsheet.open('test.xls') sheet1 = book.worksheet 'TEST' puts sheet1.row_count sheet1.delete_row(1) puts sheet1.row_count

output: $ ruby test.rb 3
3

zdavatz commented 7 years ago

Please provide me with a standalone test-script and the original source files so I can test this.

runephilosof commented 7 years ago

I really thought I had created the best possible kind of test-script by including the test in your own test script test/data/test_delete_row.xls in #137

You could even see the test script running correctly (and failing as it was supposed to) on Travis.

zdavatz commented 7 years ago

Create a separate script please with a test file so I can test separately.

daveinman commented 7 years ago

Any progress with this issue? I still see it in spreadsheet 1.1.4 under ruby 2.4.0 on Mac OS 10.12.4. When diving into the code with pry, I see that the Worksheet#delete_row method (from .../spreadsheet-1.1.4/lib/spreadsheet/worksheet.rb) references a instance variable, @rows, that is an empty array. Predictably, since this @rows variable is empty, writing the file out after the call to delete_row and reading that data in (as has been suggested) has no effect. Like runphilosof and andrecnogueira I am working with an Excel spreadsheet I have read.

(A side note: there is another worksheet.rb for excel in the source. I was surprised that pry stepped into the non-excel worksheet.rb file and wonder if that explains why @rows was empty.)

A work-around would be nice too. I have tried copying data from the Excel data I read into a spreadsheet created on-the-fly with some success deleting rows, but this loses a lot of formatting data that I don't know how to restore.

zdavatz commented 7 years ago

I can not help without getting a separate test script.

runephilosof commented 7 years ago

I see separate test scripts from three people in this discussion https://github.com/zdavatz/spreadsheet/issues/139#issuecomment-118408209 https://github.com/zdavatz/spreadsheet/issues/139#issuecomment-145282060 https://github.com/zdavatz/spreadsheet/issues/139#issuecomment-229668079

It is understandable if you don't have time to do anything about this problem. Maybe the workaround mentioned in https://github.com/zdavatz/spreadsheet/issues/139#issuecomment-145914215 (writing and rereading) works for deleting rows, I haven't tested that. Maybe GUIDE.md should be updated with this limitation.

zdavatz commented 7 years ago

Thanks, please let me know if it works for you, then I will adapt the README.