isaacharrisholt / quiffen

Quiffen is a Python package for parsing QIF (Quicken Interchange Format) files.
MIT License
33 stars 28 forks source link

Handle unknown account types #94

Open lcmcninch opened 2 days ago

lcmcninch commented 2 days ago

Apologies for creating a few issues. It looks like the QIF files I'm getting have some quirks that haven't been encountered yet. I appreciate you taking the time to consider my suggestions.

Problem

My QIF files have some account types that aren't currently part of the AccountType enum, which causes parsing to fail in Account.from_list. For example:

!Account
NSome Mutual Fund
TMutual
^
NSome Portfolio Account
TPort
^

Oddly, transactions for these same accounts fall under the !Type:Invst heading later in the file. I'm going to assume this has something to do with the !Option:AutoSwitch strangeness because the snippet above is between two of those lines, whereas the more normal ones are later in the file where the actual transactions for the account are defined.

The AccountType enum seems to reflect the QIF specification on Wikipedia, so even though these account types are coming from Quicken, I'd be hesitant to add them to the enum because that wouldn't be sustainable. Who knows how many undocumented account types there are.

Suggested Solution

See the diff in my fork as a demonstration of my suggestion: https://github.com/isaacharrisholt/quiffen/compare/main...lcmcninch:quiffen:handle_unknown_account_type

My suggestion is to an "Unknown" type to AccountType. When parsing a file, if an account type that isn't already in the enum is encountered, it will be assigned the Unknown type. This makes the parser robust to whatever type it might get without having to add new types that aren't in the spec as they come along. The user loses some information this way (i.e. the unknown account types), but as a user I'd prefer cleaning that up in my application after having read the QIF file, rather than having to clean up the QIF file manually so that quiffen can parse it.

Let me know what you think. If this works for you I can submit a PR.

isaacharrisholt commented 2 days ago

Looks sensible. It's times like this when I miss tagged unions 😅

lcmcninch commented 2 days ago

Okay, I'll make another pull request.