Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix number formatting when cells are empty #602

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ldodds
Copy link

@ldodds ldodds commented Nov 7, 2023

If a spreadsheet has empty cells formatted as numbers then attempting to turn the worksheet into CSV fails with an exception:

	27: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/formatters/csv.rb:12:in `to_csv'
	26: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/formatters/csv.rb:24:in `write_csv_content'
	25: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx.rb:134:in `first_row'
	24: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet.rb:65:in `first_row'
	23: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet.rb:126:in `first_last_row_col'
	22: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet.rb:22:in `cells'
	21: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:20:in `cells'
	20: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:214:in `extract_cells'
	19: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:214:in `with_index'
	18: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `each'
	17: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `upto'
	16: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:235:in `block in each'
	15: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:215:in `block in extract_cells'
	14: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:215:in `with_index'
	13: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `each'
	12: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `upto'
	11: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:235:in `block in each'
	10: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:224:in `block (2 levels) in extract_cells'
	 9: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:101:in `cell_from_xml'
	 8: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `each'
	 7: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:234:in `upto'
	 6: from /home/ldodds/.rvm/gems/ruby-2.7.6/gems/nokogiri-1.15.4-x86_64-linux/lib/nokogiri/xml/node_set.rb:235:in `block in each'
	 5: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:114:in `block in cell_from_xml'
	 4: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:172:in `create_cell_from_value'
	 3: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/sheet_doc.rb:172:in `new'
	 2: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/cell/number.rb:16:in `initialize'
	 1: from /home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/cell/number.rb:26:in `create_numeric'
/home/ldodds/.rvm/gems/ruby-2.7.6/bundler/gems/roo-7732ee32d43b/lib/roo/excelx/cell/number.rb:26:in `Float': invalid value for Float(): "" (ArgumentError)

The Number class does not check whether the provided cell value is empty before attempting to parse it into a Float or format it back into a String.

In our example, the given cells in the spreadsheet had an empty String as their value, and an excelx_value of [:numeric_or_formula, "#,##0.00"].

This PR

  • adds a minimal test for the Number class, for its constructor and formatted_value methods, which will demonstrate the bug
  • a fix to create_numeric to return nil if the value is empty or nil
  • a fix to formatted_value to return "" if the cell_value is empty or nil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant