#1660: Fix ExcelControl copying of stringified attribute #1661
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Description As described in frictionlessdata#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`. This also inclues a unit test to verify the change. # 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%] ```
…action with schema - fixes ExcelControl(stringified=True) documentation could clarify interaction with schemas frictionlessdata#1659
pierrecamilleri
approved these changes
Sep 5, 2024
|
I am not sure why the CI checks were not performed. @richardt-engineb, would you mind pushing an empty commit to see if it triggers the CI ? |
Trivial comment update to hopefully trigger the CI.
|
Trivial comment update commit added. Hopefully this will trigger it! Also might need approval from a maintainer as I'm a first time contributor from a public fork. |
|
It worked :) Thanks again |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
As described in #1660, the
stringifiedattribute is not copied during theto_copy()as it is not included in themetadata_profile_patch.This commit fixes it by adding it to the
metadata_profile_patchand updates the documentation to clarify the interaction ofstringifiedand schemas. This also inclues a unit test to verify the bug fix.ExcelControlattributestringifiedgets lost duringto_copy()#1660Tests
Ensure the new unit test detects the issue before the fix:
Ensure the unit test runs and passes after the fix: