search

kedder / ofxstatement

Tool to convert proprietary bank statement to OFX format, suitable for importing to GnuCash or other personal finance applications.
GNU General Public License v3.0
270 stars 61 forks source link

Add InvestStatmentLine as possible return from parse_record to fix typing check errors. #263

Open jaik03 opened 6 months ago

jaik03 commented 6 months ago

A StatementParser parse_record call should return either a StatementLine or InvestStatementLine upon call. Added missing InvestStatementLine type to return to silence typing check errors.

kedder commented 5 months ago

Oh, I forgot about this. Basically we need to find a better way here.

I think parse_record() should return StatementLine and nothing else. There should be a different method to return InvestStatementLine. I was planning to look closer at it, but forgot.

coveralls commented 5 months ago

Coverage Status

coverage: 96.05% (+0.3%) from 95.74% when pulling 9688f8fc781879c31b7cb05873fec6282407eaf0 on jaik03:AddInvestStatementLineType into 0e200177c3845e5c1b19a0c86e046fadc3c63387 on kedder:master.

jaik03 commented 5 months ago

Oh, I forgot about this. Basically we need to find a better way here.

I think parse_record() should return StatementLine and nothing else. There should be a different method to return InvestStatementLine. I was planning to look closer at it, but forgot.

Right, this was on the back burner for me and I was just fixing it to build correctly before I started digging into a better solution. I also set up a separate test environment to run through this properly so I'm not spamming you with Github workflow failures, but accidentally pushed to the wrong repo (Sorry!). Based on what you wrote I'll take a look and see if I can see any solutions that jump out at me.

jaik03 commented 3 months ago

So I finally got around to looking at this again. I duplicated out parse_record into a parse_invest_record, and it seems to work just fine with the plugin I'm developing.

The only part I'm unsure on is having the 'NotImplementedError' included. Since it's up to the plugin author to declare parse(), it'll be incumbent on them to declare either parse_record or parse_invest_record as appropriate, but it won't throw a typing error.