Skip to content

Commit

Permalink
#1660: Fix ExcelControl copying of stringified attribute (#1661)
Browse files Browse the repository at this point in the history
# Description

As described in #1660, the `stringified` attribute is not copied during
the `to_copy()` as it is not included in the `metadata_profile_patch`.

This commit fixes it by adding it to the `metadata_profile_patch` and
updates the documentation to clarify the interaction of `stringified`
and schemas. This also inclues a unit test to verify the bug fix.

- fixes #1660 
- fixes #1659 

# Tests

Ensure the new unit test detects the issue before the fix:

```
> pytest frictionless/formats/excel/__spec__/test_control.py::test_excel_control_to_copy

[...]

>       assert control_copy == control_with_changed_attributes
E       AssertionError: assert {'type': 'exc...tError': True} == {'type': 'exc...tError': True}
E
E         Omitting 8 identical items, use -vv to show
E         Differing attributes:
E         ['stringified']
E
E         Drill down into differing attribute stringified:
E           stringified: False != True

frictionless/formats/excel/__spec__/test_control.py:25: AssertionError
```

Ensure the unit test runs and passes after the fix:

```
> pytest frictionless/formats/excel/__spec__/test_control.py::test_excel_control_to_copy -v

[...]

frictionless/formats/excel/__spec__/test_control.py::test_excel_control_to_copy PASSED [100%]
```
  • Loading branch information
richardt-engineb authored Sep 6, 2024
1 parent bc67dee commit aee8c65
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
17 changes: 17 additions & 0 deletions frictionless/formats/excel/__spec__/test_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,20 @@
def test_excel_dialect():
with Resource("data/table.xlsx") as resource:
assert isinstance(resource.dialect.get_control("excel"), formats.ExcelControl)

def test_excel_control_to_copy():
"""
Test that the ExcelControl and all its attributes are correctly copied
"""
# Make a control with all values changed from the defaults
control_with_changed_attributes = formats.ExcelControl(
sheet="non-default",
fill_merged_cells=True,
preserve_formatting=True,
adjust_floating_point_error=True,
stringified=True
)

control_copy = control_with_changed_attributes.to_copy()

assert control_copy == control_with_changed_attributes
7 changes: 7 additions & 0 deletions frictionless/formats/excel/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ class ExcelControl(Control):
"""
Stringifies all the cell values. Default value
is False.
Note that a table resource schema will still be applied and types coerced to match the schema
(either provided or inferred) _after_ the rows are read as strings.
To return all cells as strings then both set `stringified=True` and specify a
schema that defines all fields to be of type string (see #1659).
"""

# Metadata
Expand All @@ -61,5 +67,6 @@ class ExcelControl(Control):
"fillMergedCells": {"type": "boolean"},
"preserveFormatting": {"type": "boolean"},
"adjustFloatingPointError": {"type": "boolean"},
"stringified": {"type": "boolean"},
},
}

0 comments on commit aee8c65

Please sign in to comment.