[16.0][FIX] account_statement_import_sheet_file : fix combination of no_header and header_line_skip_counts #834
Conversation
|
Hi @alexey-pelykh, |
| csv_or_xlsx = reader(StringIO(decoded_file), **csv_options) | ||
| header = self.parse_header(csv_or_xlsx, mapping) | ||
| csv = reader(StringIO(decoded_file), **csv_options) | ||
| csv_or_xlsx, csv_iterator_copy = itertools.tee(csv) |
There was a problem hiding this comment.
Hello @cvinh
I worked again on our issue and I've found an other way to fix the problem.
Instead of changing the logic behind no_header and header_lines_skip_count (because I realized that our previous changes would break all the already existing configurations), I've modified the way the headers are retrieved from a csv file.
The previous issue was that the csv iterator was consumed when the headers were parsed. Now that I use itertools.tee, I create a copy of the csv iterator to retrieve the headers, and I use another one to parse the rows.
Here's how I've done my PR :
I've forked the repo, cherry picked your changes (to have the tests) and made them work with this new logic. I'm embarrassed that you don't get credits for all the tests you've written. Maybe there's another way to keep you author of the tests ?
Please let me know if it's clear and if this solution suits you.
Best regards
There was a problem hiding this comment.
Cc @cvinh
I hope you are well :)
Can you check this alternative and tell me what you think about it please ? Many thanks
There was a problem hiding this comment.
Hi @mondot Thanks for working on that ! I'm sorry to be late
Can you please keep my commit (that includes both new tests and refactorization of the code), it will be easier to review
At the end, we would have 2 commits but I think it's worth keeping them because other guys who are migrating to v17 and v18 will need the history
Allow to import csv files with sheet mapping being configured with no_header and some header_lines_skip_count
mondot
force-pushed
the
16.0-fix-account_statement_import_sheet_file-header_lines_skip_count
branch
from
7b8477f to
7ceea10
Compare
mondot
changed the title
3 participants
Allow to import csv files with sheet mapping being configured with no_header and some header_lines_skip_count