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

Fixes #578 - Do not mutate data in place when applying formatters #579

Merged
merged 2 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 19 additions & 17 deletions src/tablib/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ def append(self, value):
def insert(self, index, value):
self._row.insert(index, value)

def copy(self):
return Row(self._row.copy(), self.tags.copy())

def __contains__(self, item):
return item in self._row

Expand Down Expand Up @@ -270,27 +273,25 @@ def _package(self, dicts=True):

_data = list(self._data)

# Execute formatters
if self._formatters:
for row_i, row in enumerate(_data):
def format_row(row):
# Execute formatters
if self._formatters:
row = row.copy() # To not mutate internal data structure
for col, callback in self._formatters:
try:
if col is None:
for j, c in enumerate(row):
_data[row_i][j] = callback(c)
else:
_data[row_i][col] = callback(row[col])
except IndexError:
raise InvalidDatasetIndex
if col is None:
# Apply formatter to all cells
row = [callback(cell) for cell in row]
else:
row[col] = callback(row[col])
return list(row)

if self.headers:
if dicts:
data = [dict(list(zip(self.headers, data_row))) for data_row in _data]
data = [dict(list(zip(self.headers, format_row(row)))) for row in _data]
else:
data = [list(self.headers)] + list(_data)
data = [list(self.headers)] + [format_row(row) for row in _data]
else:
data = [list(row) for row in _data]

data = [format_row(row) for row in _data]
return data

def _get_headers(self):
Expand Down Expand Up @@ -622,7 +623,8 @@ def get_col(self, index):
def add_formatter(self, col, handler):
"""Adds a formatter to the :class:`Dataset`.

:param col: column to. Accepts index int or header str.
:param col: column to. Accepts index int, header str, or None to apply
the formatter to all columns.
:param handler: reference to callback function to execute against
each cell value.
"""
Expand All @@ -633,7 +635,7 @@ def add_formatter(self, col, handler):
else:
raise KeyError

if not col > self.width:
if col is None or col <= self.width:
self._formatters.append((col, handler))
else:
raise InvalidDatasetIndex
Expand Down
28 changes: 25 additions & 3 deletions tests/test_tablib.py
Original file line number Diff line number Diff line change
Expand Up @@ -564,12 +564,34 @@ def test_formatters(self):
"""Confirm formatters are being triggered."""

def _formatter(cell_value):
return str(cell_value).upper()
return str(cell_value)[1:]

self.founders.add_formatter('last_name', _formatter)

for name in [r['last_name'] for r in self.founders.dict]:
self.assertTrue(name.isupper())
expected = [
{'first_name': 'John', 'last_name': 'dams', 'gpa': 90},
{'first_name': 'George', 'last_name': 'ashington', 'gpa': 67},
{'first_name': 'Thomas', 'last_name': 'efferson', 'gpa': 50},
]
self.assertEqual(self.founders.dict, expected)
# Test once more as the result should be the same
self.assertEqual(self.founders.dict, expected)

def test_formatters_all_cols(self):
"""
Passing None as first add_formatter param apply formatter to all columns.
"""

def _formatter(cell_value):
return str(cell_value).upper()

self.founders.add_formatter(None, _formatter)

self.assertEqual(self.founders.dict, [
{'first_name': 'JOHN', 'last_name': 'ADAMS', 'gpa': '90'},
{'first_name': 'GEORGE', 'last_name': 'WASHINGTON', 'gpa': '67'},
{'first_name': 'THOMAS', 'last_name': 'JEFFERSON', 'gpa': '50'},
])

def test_unicode_renders_markdown_table(self):
# add another entry to test right field width for
Expand Down