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

Handle writing tuples #153

Closed
steveoh opened this Issue Sep 3, 2014 · 9 comments

Comments

2 participants
@steveoh

steveoh commented Sep 3, 2014

I am working with spatial data and the shape format for a point is an x, y tuple. In the worksheet class, the write method tries all of the options to write this tuple and falls through to the part where it tries a float. With my code I get a TypeError instead of a ValueError and the application busts without trying the string style. I commented out the float casting and it doesn't bust on write_row but then raises a separate error when you close the workbook.

Traceback (most recent call last):
File "", line 1, in
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\workbook.py", line 246, in close
self._store_workbook()
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\workbook.py", line 448, in _store_workbook
xml_files = packager._create_package()
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\packager.py", line 140, in _create_package
self._write_shared_strings_file()
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\packager.py", line 266, in _write_shared_strings_file
sst._assemble_xml_file()
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\sharedstrings.py", line 53, in _assemble_xml_file
self._write_sst_strings()
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\sharedstrings.py", line 83, in _write_sst_strings
self._write_si(string)
File "C:\Python27\ArcGIS10.2\lib\site-packages\xlsxwriter\sharedstrings.py", line 95, in _write_si
string = re.sub('(x[0-9a-fA-F]{4})', r'_x005F\1', string)
File "C:\Python27\ArcGIS10.2\Lib\re.py", line 151, in sub
return _compile(pattern, flags).sub(repl, string, count)
TypeError: expected string or buffer

is there a way to successfully store tuples or is it expected that I format them how I want them before writing them to the sheet?

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Sep 3, 2014

Owner

Hi Steve,

Can you give a small working (or maybe in this case non-working) example of what you are trying to do and perhaps a small ascii example of what you expect the output to look like.

Regards,

John

Owner

jmcnamara commented Sep 3, 2014

Hi Steve,

Can you give a small working (or maybe in this case non-working) example of what you are trying to do and perhaps a small ascii example of what you expect the output to look like.

Regards,

John

@steveoh

This comment has been minimized.

Show comment
Hide comment
@steveoh

steveoh Sep 3, 2014

xls = xlsxwriter.Workbook('test.xlsx')
book = xls.add_worksheet('test_tuple')
book.write_row('A1', [(1, 2)])
xls.close()

I think to be safe I would want the cell to be '(1, 2)' as a string via str((1,2)) or something. I can't think of how else you would want to represent this.

steveoh commented Sep 3, 2014

xls = xlsxwriter.Workbook('test.xlsx')
book = xls.add_worksheet('test_tuple')
book.write_row('A1', [(1, 2)])
xls.close()

I think to be safe I would want the cell to be '(1, 2)' as a string via str((1,2)) or something. I can't think of how else you would want to represent this.

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Sep 3, 2014

Owner

Hi Steve,

Thanks for the example.

The write() and write_row() methods are mainly intended as utility functions to map to Excel types from corresponding Python types. Since there isn't a tuple type in Excel then it is up to the user to convert it to the available type that they want.

In your case if you want a string then you should probably just convert it using str() as you show above.

I would probably use a list comprehension:

mylist = [(1, 2)]
mylist = [str(t) for t in mylist]

# or if there are other data types in the list:

mylist = [str(t) if type(t) is tuple else t for t in mylist]

worksheet.write_row('A1', mylist)
Owner

jmcnamara commented Sep 3, 2014

Hi Steve,

Thanks for the example.

The write() and write_row() methods are mainly intended as utility functions to map to Excel types from corresponding Python types. Since there isn't a tuple type in Excel then it is up to the user to convert it to the available type that they want.

In your case if you want a string then you should probably just convert it using str() as you show above.

I would probably use a list comprehension:

mylist = [(1, 2)]
mylist = [str(t) for t in mylist]

# or if there are other data types in the list:

mylist = [str(t) if type(t) is tuple else t for t in mylist]

worksheet.write_row('A1', mylist)
@steveoh

This comment has been minimized.

Show comment
Hide comment
@steveoh

steveoh Sep 3, 2014

I am using

def convert_tuple(value):
            if not isinstance(value, tuple):
                return value

            return str(value)

row = map(convert_tuple, row)
worksheet.write_row('A2', row)

I figured this might be your response. I would have just hoped for a better error message.

steveoh commented Sep 3, 2014

I am using

def convert_tuple(value):
            if not isinstance(value, tuple):
                return value

            return str(value)

row = map(convert_tuple, row)
worksheet.write_row('A2', row)

I figured this might be your response. I would have just hoped for a better error message.

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Sep 3, 2014

Owner
def convert_tuple(value):
             if not isinstance(value, tuple):
                 return value

            return str(value)

If you had started with that instead of a stack trace it would have saved some time. :-)

I would have just hoped for a better error message.

Yes. You are probably right about that.

Looking at the logic of the write() code again I would almost be tempted to pass on all failed float() conversions in the section of code you highlighted.

This would probably have done the right thing for your case.

On the other hand it might silently work for other cases where people didn't expect it to.

In the end I think that the write() API is pretty clear about type handling so I leave it as it is.

Regards,

John

Owner

jmcnamara commented Sep 3, 2014

def convert_tuple(value):
             if not isinstance(value, tuple):
                 return value

            return str(value)

If you had started with that instead of a stack trace it would have saved some time. :-)

I would have just hoped for a better error message.

Yes. You are probably right about that.

Looking at the logic of the write() code again I would almost be tempted to pass on all failed float() conversions in the section of code you highlighted.

This would probably have done the right thing for your case.

On the other hand it might silently work for other cases where people didn't expect it to.

In the end I think that the write() API is pretty clear about type handling so I leave it as it is.

Regards,

John

@steveoh

This comment has been minimized.

Show comment
Hide comment
@steveoh

steveoh Sep 3, 2014

that convert tuple is my work around to make it run. The stack trace is from passing in a tuple. I am happy with you leaving it as is. The error message would be the enhancement then. #154

steveoh commented Sep 3, 2014

that convert tuple is my work around to make it run. The stack trace is from passing in a tuple. I am happy with you leaving it as is. The error message would be the enhancement then. #154

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Sep 4, 2014

Owner

In general I don't find stack traces very useful in bug reports (in my own testing yes but not in bug reports). Also in this case the stack trace was from modified code so it is even a little less useful.

The unmodified code raises a TypeError exception which is what the API says it should do so I'd still prefer to leave this unchanged.

Update: potential fix below.

P.S. I didn't see your tweets until several hours after the open issue. I wasn't ignoring you. :-)

Owner

jmcnamara commented Sep 4, 2014

In general I don't find stack traces very useful in bug reports (in my own testing yes but not in bug reports). Also in this case the stack trace was from modified code so it is even a little less useful.

The unmodified code raises a TypeError exception which is what the API says it should do so I'd still prefer to leave this unchanged.

Update: potential fix below.

P.S. I didn't see your tweets until several hours after the open issue. I wasn't ignoring you. :-)

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Sep 4, 2014

Owner

What would you think of something like this:

diff --git a/xlsxwriter/worksheet.py b/xlsxwriter/worksheet.py
index e0090bb..e90dda6 100644
--- a/xlsxwriter/worksheet.py
+++ b/xlsxwriter/worksheet.py
@@ -408,6 +408,8 @@ class Worksheet(xmlwriter.XMLwriter):
                 return self.write_number(row, col, f, *args[1:])
         except ValueError:
             pass
+        except TypeError:
+            raise TypeError("Unsupported type %s in write()" % type(token))

         # Finally try string.
         try:

Which for your code example above would give:

$ python ex01.py
Traceback (most recent call last):
  File "ex01.py", line 5, in <module>
    book.write_row('A1', [(1, 2)])
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 61, in cell_wrapper
    return method(self, *args, **kwargs)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 978, in write_row
    error = self.write(row, col, token, cell_format)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 61, in cell_wrapper
    return method(self, *args, **kwargs)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 412, in write
    raise TypeError("Unsupported type %s in write()" % type(token))
TypeError: Unsupported type <type 'tuple'> in write()
Exception Exception: Exception('Exception caught in workbook destructor. ...
Owner

jmcnamara commented Sep 4, 2014

What would you think of something like this:

diff --git a/xlsxwriter/worksheet.py b/xlsxwriter/worksheet.py
index e0090bb..e90dda6 100644
--- a/xlsxwriter/worksheet.py
+++ b/xlsxwriter/worksheet.py
@@ -408,6 +408,8 @@ class Worksheet(xmlwriter.XMLwriter):
                 return self.write_number(row, col, f, *args[1:])
         except ValueError:
             pass
+        except TypeError:
+            raise TypeError("Unsupported type %s in write()" % type(token))

         # Finally try string.
         try:

Which for your code example above would give:

$ python ex01.py
Traceback (most recent call last):
  File "ex01.py", line 5, in <module>
    book.write_row('A1', [(1, 2)])
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 61, in cell_wrapper
    return method(self, *args, **kwargs)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 978, in write_row
    error = self.write(row, col, token, cell_format)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 61, in cell_wrapper
    return method(self, *args, **kwargs)
  File "/Users/John/Python/XlsxWriter/xlsxwriter/worksheet.py", line 412, in write
    raise TypeError("Unsupported type %s in write()" % type(token))
TypeError: Unsupported type <type 'tuple'> in write()
Exception Exception: Exception('Exception caught in workbook destructor. ...
@steveoh

This comment has been minimized.

Show comment
Hide comment
@steveoh

steveoh Sep 4, 2014

That looks awesome.

steveoh commented Sep 4, 2014

That looks awesome.

jmcnamara added a commit that referenced this issue Sep 17, 2014

Made write() exception clearer for unhandled types.
Made the exception in write() clearer when handling unsupported
types so that it now gives a more accurate TypeError instead of
a ValueError. Issue #153.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment