search

egh / ledger-autosync

Synchronize your ledger-cli files with your bank.
GNU General Public License v3.0
273 stars 42 forks source link

some OFX providers give CUSIP instead of ticker symbol #10

Closed jblachly closed 8 years ago

jblachly commented 8 years ago

@egh , thanks again for ledger-autosync. Final barrier preventing me from integrating into my workflow is that Fidelity's OFX files give the CUSIP in <SECID>, rather than the ticker symbol.

I've written a class that will translate CUSIP->Ticker (if that information is supplied in the OFX file) for both InvestmentTransactions and position statements (misnomer, actually P commodity price lines).

This is not the optimal implementation as I would have it, for several reasons:

  1. OFXparse does not actually parse and export the UNIQUEIDTYPE inside SECINFO tags, so the only way to know if it is a CUSIP (instead of a ticker symbol) is to look it up in a CUSIP->Ticker lookup table (and succeed)
  2. Any time the "security" property is accessed (for example, txn.security or pos.security) we have to go through the translation step. So far, I have solved this by putting a check and conversion at the beginning of any function using the security property. However, it would feel cleaner if we wrote a translator that iterated through the OFX object and translated all CUSIPs to Ticker symbols in a fell swoop. Another solution would be to create OFXTransaction class (and for clarity probably rename existing Transaction class to LedgerTransaction) and OFXPosition class that performed dynamic translation of the security field as requested.

All of that being said, I don't want to let the perfect be the enemy of the good and shortly I will issue a pull request that functions. Please take a look at the implementation and let me know if you object and would require re-factor.

jblachly commented 8 years ago

On second thought I have refactored this as a static conversion in cli.py (instead of multiple point-of-use conversions in converter.py). Will try to create test with sanitized test OFX for this PR. Thanks for patience.

egh commented 8 years ago

Thanks! I'll be interested to see what you come up with. To be totally honest I never noticed that there were TICKER entries in the OFX files that can be used. I've been suffering with the CUSIPs as commodities. Ticket symbols sound much nicer. :)

It shouldn't be too hard to get a new feature into ofxparse - they have been pretty good about merging my PRs.

jblachly commented 8 years ago

FYI, in addition to ticker symbols, the security/mutual fund full name or description is also included, so you could also now add transaction metadata that includes this. I probably should have. It would require making the security_list globally accessible, or passing it to the member functions of OfxConverter, which I no longer do in the current implementation (which performs its work in print_results()

egh commented 8 years ago

Thanks again. I really appreciate your fixing this!