cantino / reckon

Flexibly import bank account CSV files into Ledger for command-line accounting
http://blog.andrewcantino.com/blog/2013/02/16/command-line-accounting-with-ledger-and-reckon/
MIT License
414 stars 46 forks source link

Silently misinterprets UK dates #18

Closed purcell closed 10 years ago

purcell commented 10 years ago

Hi! I have several CSV files containing thousands of transactions with dates in the UK DD/MM/YYYY format, and have just spent the afternoon working through those transactions with reckon (which is a tremendous help, I might add).

However, I've discovered to my dismay that the dates have been silently messed up in a way which is going to be extremely difficult to fix without going through the whole process again. Here are the raw and generated dates for a couple of examples:

29/03/2013 -> 29/03/2013 CORRECT
12/07/2013 -> 07/12/2013 WRONG

It looks like any date which can be interpreted as a valid US-style "MM/DD/YYYY" date will be parsed as such, and "DD/MM/YYYY" is used as a fallback.

I would expect a hard error in the face of inconsistent input.

This is likely the fault of Ruby's liberal date parsing, but rather than risk users losing hours of time, perhaps it would be good to use a specific date format when parsing, and allow users to explicitly override it?

purcell commented 10 years ago

I see now that there is a --date-format option not mentioned in the README.

I'd suggest that reckon should indicate at start-up which date format it is using, and that it should explicitly confirm that all dates in the input file can be parsed by that single pre-selected date format. The per-line fallback scheme is a recipe for disaster, as I've unfortunately demonstrated.

purcell commented 10 years ago

As aside: I figured out that I can fix up my ledger data by looking for converted YYYY/MM/DD transactions in which both the MM and DD are <= 12, and then switching the two fields. So that's a relief, at least!

cantino commented 10 years ago

Hey, I'm sorry that this was hard to figure out-- I should have had it better documented. Once you used the --date-format option, did it always get the dates right?

Glad you're using Reckon!

purcell commented 10 years ago

I haven't had to re-run it, but from reading the code I'm sure the --date-format option will do the right thing, including barfing on invalid input. I'd still strongly suggest initializing the date format option to the US format, and just using that fixed format throughout, just to eliminate the risk of errors. It is financial data, after all. :-)

cantino commented 10 years ago

I agree. Do you want to send another PR?

On Tuesday, December 3, 2013, Steve Purcell wrote:

I haven't had to re-run it, but from reading the code I'm sure the --date-format option will do the right thing, including barfing on invalid input. I'd still strongly suggest initializing the date format option to the US format, and just using that fixed format throughout, just to eliminate the risk of errors. It is financial data, after all. :-)

— Reply to this email directly or view it on GitHubhttps://github.com/cantino/reckon/issues/18#issuecomment-29690752 .

purcell commented 10 years ago

Yeah, I was thinking I probably will. Just need to actually get done with this book-keeping first, then I'll circle back round to it. :-)

In the meantime, thanks again for reckon.

cantino commented 10 years ago

That sounds good, thanks! Let me know if you have any questions around the PR!

purcell commented 10 years ago

Shouldn't be a problem preparing a PR, but it likely won't happen for at least another week. It's on my list. :-)

cantino commented 10 years ago

Haha, no worries. Just checking in!

BlackEdder commented 10 years ago

This bug should now be fixed and reckon will throw an error on date formats that can be interpreted in multiple ways. See pull request #32 and #35 for more details.