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

Error bars require that unused parameters are set to None #115

Closed
abirmingham opened this Issue Apr 2, 2014 · 4 comments

Comments

2 participants
@abirmingham

abirmingham commented Apr 2, 2014

xlsxwriter/chart.py contains the following in _write_custom_error:

        if error_bars['plus_values']:
            # Write the c:plus element.
            self._xml_start_tag('c:plus')

            if isinstance(error_bars['plus_values'], list):
                self._write_num_lit(error_bars['plus_values'])
            else:
                self._write_num_ref(error_bars['plus_values'],
                                    error_bars.get('plus_data'),
                                    'num')
            self._xml_end_tag('c:plus')

        if error_bars['minus_values']:
            # Write the c:minus element.
            self._xml_start_tag('c:minus')

            if isinstance(error_bars['minus_values'], list):
                self._write_num_lit(error_bars['minus_values'])
            else:
                self._write_num_ref(error_bars['minus_values'],
                                    error_bars['minus_data'],
                                    'num')
            self._xml_end_tag('c:minus')

Because bracket notation is used to lookup values on the dicts, an exception is raised in many cases where attributes can be (legitimately) missing. For example, the following code will raise a KeyError on 'plus_data':

chart.add_series({
    'values': '=Sheet1!$A$1:$A$6',
    'y_error_bars': {
        'type':         'custom',
        'plus_values':  '=Sheet1!$B$1:$B$6'
    },
})

If we set 'plus_data' to None, then we move on to a 'minus_values' KeyError, after which we'll need to set 'minus_data' as well. Changing the bracket notation to .get()'s appears to solve the issue.

I'm curious about two things:

  1. Is there some reason we should be explicitly setting 3 extra attributes to None in this case?
  2. What are plus_data and minus_data used for? I don't see them in the docs.

Thanks!

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Apr 2, 2014

Owner

Hi Alex,

Nice to hear from you again.

The minus_data is optional so it should be error_bars.get('minus_data') like the plus data. That looks like a bug.

Could you flesh out your code into a working example that demonstrates the issue so that it is clear that a fix is fixing the right thing.

As to your questions:

  1. Related to the above bug I think.
  2. Those values are undocumented on purpose since from a users point of view they are optional, and would be a pain to have to supply each time. They are there to supply cached data to Excel for 100% fidelity with the file format. We discussed that back in issue #17 I think. :-)

John

Owner

jmcnamara commented Apr 2, 2014

Hi Alex,

Nice to hear from you again.

The minus_data is optional so it should be error_bars.get('minus_data') like the plus data. That looks like a bug.

Could you flesh out your code into a working example that demonstrates the issue so that it is clear that a fix is fixing the right thing.

As to your questions:

  1. Related to the above bug I think.
  2. Those values are undocumented on purpose since from a users point of view they are optional, and would be a pain to have to supply each time. They are there to supply cached data to Excel for 100% fidelity with the file format. We discussed that back in issue #17 I think. :-)

John

@abirmingham

This comment has been minimized.

Show comment
Hide comment
@abirmingham

abirmingham Apr 2, 2014

Hey John -

Thanks for the quick response. Apologies for not responding sooner to your questions in #17. Here is an example using plus_values:

import xlsxwriter

workbook = xlsxwriter.Workbook('chart_line.xlsx')
worksheet = workbook.add_worksheet()

# Add the worksheet data to be plotted.
data = [10, 40, 50, 20, 10, 50]
worksheet.write_column('A1', data)

# Add error bar series
data = [5, 5, 5, 5, 5, 5]
worksheet.write_column('B1', data)

# Create a new chart object.
chart = workbook.add_chart({'type': 'line'})

# Add a series to the chart.
# In this case I'd prefer not to specify the attributes with None values
chart.add_series({
    'values': '=Sheet1!$A$1:$A$6',
    'y_error_bars': {
        'type':         'custom',
        'plus_values':  '=Sheet1!$B$1:$B$6',
        'plus_data': None,    
        'minus_values': None,
        'minus_data': None
    },
})

# Insert the chart into the worksheet.
worksheet.insert_chart('C1', chart)

workbook.close()

Let me know if you were looking for something different than this.

Thanks!

abirmingham commented Apr 2, 2014

Hey John -

Thanks for the quick response. Apologies for not responding sooner to your questions in #17. Here is an example using plus_values:

import xlsxwriter

workbook = xlsxwriter.Workbook('chart_line.xlsx')
worksheet = workbook.add_worksheet()

# Add the worksheet data to be plotted.
data = [10, 40, 50, 20, 10, 50]
worksheet.write_column('A1', data)

# Add error bar series
data = [5, 5, 5, 5, 5, 5]
worksheet.write_column('B1', data)

# Create a new chart object.
chart = workbook.add_chart({'type': 'line'})

# Add a series to the chart.
# In this case I'd prefer not to specify the attributes with None values
chart.add_series({
    'values': '=Sheet1!$A$1:$A$6',
    'y_error_bars': {
        'type':         'custom',
        'plus_values':  '=Sheet1!$B$1:$B$6',
        'plus_data': None,    
        'minus_values': None,
        'minus_data': None
    },
})

# Insert the chart into the worksheet.
worksheet.insert_chart('C1', chart)

workbook.close()

Let me know if you were looking for something different than this.

Thanks!

@jmcnamara jmcnamara added the bug label Apr 2, 2014

@jmcnamara jmcnamara self-assigned this Apr 2, 2014

@abirmingham

This comment has been minimized.

Show comment
Hide comment
@abirmingham

abirmingham Apr 3, 2014

FYI I am using the following patch temporarily. Let me know if you want me to create a pull request with the same:

diff --git a/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py b/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
index 206f68f..0838ad4 100644
--- a/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
+++ b/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
@@ -3283,7 +3283,7 @@ class Chart(xmlwriter.XMLwriter):
     def _write_custom_error(self, error_bars):
         # Write the custom error bars tags.

-        if error_bars['plus_values']:
+        if error_bars.get('plus_values'):
             # Write the c:plus element.
             self._xml_start_tag('c:plus')

@@ -3291,11 +3291,11 @@ class Chart(xmlwriter.XMLwriter):
                 self._write_num_lit(error_bars['plus_values'])
             else:
                 self._write_num_ref(error_bars['plus_values'],
-                                    error_bars['plus_data'],
+                                    error_bars.get('plus_data'),
                                     'num')
             self._xml_end_tag('c:plus')

-        if error_bars['minus_values']:
+        if error_bars.get('minus_values'):
             # Write the c:minus element.
             self._xml_start_tag('c:minus')

@@ -3303,7 +3303,7 @@ class Chart(xmlwriter.XMLwriter):
                 self._write_num_lit(error_bars['minus_values'])
             else:
                 self._write_num_ref(error_bars['minus_values'],
-                                    error_bars['minus_data'],
+                                    error_bars.get('minus_data'),
                                     'num')
             self._xml_end_tag('c:minus')

abirmingham commented Apr 3, 2014

FYI I am using the following patch temporarily. Let me know if you want me to create a pull request with the same:

diff --git a/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py b/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
index 206f68f..0838ad4 100644
--- a/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
+++ b/vendor/XlsxWriter-0.5.3/xlsxwriter/chart.py
@@ -3283,7 +3283,7 @@ class Chart(xmlwriter.XMLwriter):
     def _write_custom_error(self, error_bars):
         # Write the custom error bars tags.

-        if error_bars['plus_values']:
+        if error_bars.get('plus_values'):
             # Write the c:plus element.
             self._xml_start_tag('c:plus')

@@ -3291,11 +3291,11 @@ class Chart(xmlwriter.XMLwriter):
                 self._write_num_lit(error_bars['plus_values'])
             else:
                 self._write_num_ref(error_bars['plus_values'],
-                                    error_bars['plus_data'],
+                                    error_bars.get('plus_data'),
                                     'num')
             self._xml_end_tag('c:plus')

-        if error_bars['minus_values']:
+        if error_bars.get('minus_values'):
             # Write the c:minus element.
             self._xml_start_tag('c:minus')

@@ -3303,7 +3303,7 @@ class Chart(xmlwriter.XMLwriter):
                 self._write_num_lit(error_bars['minus_values'])
             else:
                 self._write_num_ref(error_bars['minus_values'],
-                                    error_bars['minus_data'],
+                                    error_bars.get('minus_data'),
                                     'num')
             self._xml_end_tag('c:minus')

jmcnamara added a commit that referenced this issue May 4, 2014

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara May 4, 2014

Owner

Hi Alex,

Fixed in version 0.5.4 on PyPi,

Thanks for the report.

John

Owner

jmcnamara commented May 4, 2014

Hi Alex,

Fixed in version 0.5.4 on PyPi,

Thanks for the report.

John

@jmcnamara jmcnamara closed this May 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment