Merge lp:~wolfer7/wxbanker/bugfix-lp-534923 into lp:wxbanker

Proposed by Michael Rooney
Status: Merged
Merged at revision: not available
Proposed branch: lp:~wolfer7/wxbanker/bugfix-lp-534923
Merge into: lp:wxbanker
Diff against target: 0 lines
To merge this branch: bzr merge lp:~wolfer7/wxbanker/bugfix-lp-534923
Reviewer Review Type Date Requested Status
Michael Rooney Approve
Review via email: mp+21107@code.launchpad.net

This proposal supersedes a proposal from 2010-03-09.

Description of the change

changed csv-importer. previously one could only skip the first line, now it is possible to exactly specify how many rows should be skipped at the beginning of a file.

also added 'comdirect' to the shipped csv profiles.

To post a comment you must log in.
Revision history for this message
Michael Rooney (mrooney) wrote : Posted in a previous version of this proposal

Looks good, thanks for this feature and merge request! I can happily merge this after a few quick things. I would have just fixed the first two myself and pointed them out for your reference but the third one requires data from you.

 - It looks like there is a typo: "Linex to skip"
 - Instead of doing the counting logic like "if settings['linesToSkip']>linesSkipped: linesSkipped+=1", I would remove that logic and just change the for to "for row in csvReader[settings['linesToSkip']:]:"
 - All the included currencies should have tests with a small fixture CSV. It is fairly straightforward, see tests/csvimportertests.py for test examples, and you'll want to put an example CSV with a few transactions in the data/fixtures directory with the other .csv files.

Let me know if you have any questions and I look forward to merging this, thanks again!

review: Needs Fixing
Revision history for this message
wolfer (wolfer7) wrote : Posted in a previous version of this proposal

Thanks, for merging this. This significantly improves my wxbanker experience, cause I don't need to manually adjust my csv files anymore. I just did some fixes.

1. just a typo
2. csvReader[settings['linesToSkip']] does not work:
      csvimporter.py", line 86, in getTransactionsFromCSV
      for row in csvReader[settings['linesToSkip']]:
      TypeError: '_csv.reader' object is unsubscriptable
3. I added a sample csv to the data directory and also a test function. Seems to work.

I had some problems running the unit tests, so I added a simple test.py to the main directory. Unfortunately localetests.py crashes..

Revision history for this message
Michael Rooney (mrooney) wrote :

Okay, I merged it into trunk and made a few minor tweaks in r607 just to better document the logic. I also removed the test.py file; the localetests depend on having fr_FR and ru_RU I believe available, but you can also just run a specific suite via: "python -m wxbanker/tests/csvimportertests", for future reference.

Thanks again for the improvement and csv profile! If you are feeling adventurous feel free to take a peek at https://blueprints.launchpad.net/wxbanker/+spec/csv-import-2.0 , which has some low-hanging usability papercuts, but also some improvements that would fix bugs and the general "automatic csv parsing" idea/logic.

review: Approve

Preview Diff

Empty

Subscribers

People subscribed via source and target branches

to all changes: