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.

Sign up for GitHub

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

Jump to bottom

Ensure external select to csv conversion works as expected #2349

Merged
merged 4 commits into from
Jan 20, 2023
Merged

Ensure external select to csv conversion works as expected #2349

merged 4 commits into from
Jan 20, 2023

Conversation

DavisRayM
Copy link
Contributor

@DavisRayM DavisRayM commented Jan 10, 2023 •
edited
Loading

Changes / Features implemented

  • Update sheet_to_csv function; Ensure that cells are only processed if they have a value
  • Ensure all cells are checked and converted to data/integer when applicable
This PR sorts an issue where an empty column is falsely diagnosed as an integer field which caused the XLSX to be processed for integer & datetime fields.

Note: The integer and datetime processing functionality was broken since it treated the sheet as a 0-based index array of which it's not. https://openpyxl.readthedocs.io/en/latest/api/openpyxl.worksheet.worksheet.html#openpyxl.worksheet.worksheet.Worksheet.iter_rows

Steps taken to verify this change does what is intended

  • Updated Tests
  • QA

Side effects of implementing this change

N/A

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

Closes https://github.com/onaio/zebra/issues/7312

@DavisRayM DavisRayM marked this pull request as ready for review January 17, 2023 10:21
@DavisRayM DavisRayM force-pushed the external-selects-fix branch 2 times, most recently from 8ac34f6 to 22c3c57 Compare January 17, 2023 20:34
@DavisRayM DavisRayM requested a review from ukanga January 17, 2023 20:37
@DavisRayM DavisRayM force-pushed the external-selects-fix branch 7 times, most recently from 9f1f90a to 7521274 Compare January 19, 2023 12:59
Davis Muro added 2 commits January 19, 2023 16:07
Recreate reported issue
366f063 
- Issue is caused when a row within the `list_name` column is empty
  which leads to it being diagnosed as an integer field

- Update integer name column test to validate the generated CSV
Update sheet_to_csv function
4f37461 
- Ensure headers are not duplicated
- Ensure that integers & datetimes are correctly handler.
- Ensure empty values are not processed
- Utilize PyXForm `xlsx_value_to_str` to convert XLSX values
- Update tests
@DavisRayM DavisRayM merged commit ad77467 into main Jan 20, 2023
@DavisRayM DavisRayM deleted the external-selects-fix branch January 20, 2023 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ukanga ukanga ukanga approved these changes

@ciremusyoka ciremusyoka Awaiting requested review from ciremusyoka

@FrankApiyo FrankApiyo Awaiting requested review from FrankApiyo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DavisRayM @ukanga