Add Schwab bank support to schwab_csv importer #91

bayesianmind · 2021-01-11T06:59:07Z

I have quite a lot of activity / business with Schwab including a checking account so I brought in support for everything I could find in my own Schwab CSV files. In the spirit of #88 .

Other changes in the schwab_csv source: (apologies for not splitting these out, if that's a bother I can do the extra book keeping to get them into independent pull reqs).

Added support for 7 new BrokerageActions.
Added support for fractional shares, which Schwab added last year. You can get these in your account via dividend re-investment or the fractional share purchase feature. The source previously assume int for share quantities.
Make schwab_csv post cleared status for Schwab accounts. This lets uncleared transactions properly appear in the import webui.
Make the currency configurable just in case instead of hardcoding USD.
Added a dedicated interest type that we now use consistently for Checking account interest and "Bank interest" paid on brokerage account cash balances.
Tweaked some of the key regexps to work on the files I am getting from Schwab in addition to the testfiles.

Known caveats:

The banking CSVs indicate they may contain pending transactions in a special way. I didn't have any in my file to add support for. Likely the importer will import them the same whether they are pending or not. This could result in some extraneous imports for pending auth charges that would normally drop off. If someone donates a CSV with the format it would be easy to add support for it.
Does not support corporate equity awards accounts - for Google's at least it is quite different from the Brokerage format and I had written my own importer before schwab_csv was added. If there's interest I can open source the Google equity awards source.

coveralls · 2021-01-11T07:01:49Z

Pull Request Test Coverage Report for Build 241

125 of 133 (93.98%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 0.0%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
beancount_import/source/ultipro_google_statement.py	0	3	0.0%
beancount_import/source/schwab_csv.py	125	130	96.15%

Totals
Change from base Build 234:	0.0%
Covered Lines:	0
Relevant Lines:	0

💛 - Coveralls

Zburatorul · 2021-01-12T15:20:41Z

Did you forget to include a commit with more code changes? I don't see the 7 new brokerage actions.

bayesianmind · 2021-01-12T16:42:26Z

See line 134 and on. I didn't have to add new classes for them, they're similar enough to existing types.

carljm

This looks great overall! Thanks so much for the additions and improvements. Made a few comments but overall I think this is pretty close to ready.

One thing is I noticed a few bad type annotations; haven't tested but I would think these should cause a mypy failure. Can you verify that tox is green with your changes?

beancount_import/source/schwab_csv.py

...data/source/schwab_csv/test_basic/transactions/EAC_Checking_Transactions_20201223-160009.CSV

beancount_import/source/schwab_csv.py

carljm · 2021-01-20T20:25:32Z

I should also clarify that @jbms is the author and maintainer of beancount-import and the ultimate authority on whether this gets merged; just offering my review as the initial contributor of the Schwab CSV importer.

bayesianmind · 2021-01-21T17:51:40Z

Thanks for the excellent review @carljm ! I will update this over the next few days as I have free cycles.

zhou13 · 2021-02-07T07:13:41Z

beancount_import/source/schwab_csv.py

@@ -1077,10 +1319,10 @@ def _load_positions_csv(
        symbol = STRIP_FROM_SYMBOL_RE.sub("", symbol)
        quantity = _convert_int(row["Quantity"])


The quantity of reinvested portfolios can be decimal. I need to replace this line with quantity = _convert_decimal(row["Quantity"]).

Thanks! Made that change here as well.

Also adds 6 additional BrokerageActions found it my Brokerage CSV imports.

Now reusing the same types between brokerage and banking imports.

I didn't realize I should be running tox checks and it turns out the type inference engine wasn't able to correctly infer non-nullability previously. Introducing a variable fixes the check.

bayesianmind

Thank you both so much for your feedback. This PR is way better for it.
It's ready to go now. When combined with #96 tox is passing.

...data/source/schwab_csv/test_basic/transactions/EAC_Checking_Transactions_20201223-160009.CSV

beancount_import/source/schwab_csv.py

bayesianmind · 2021-02-14T01:52:53Z

beancount_import/source/schwab_csv.py

@@ -1077,10 +1319,10 @@ def _load_positions_csv(
        symbol = STRIP_FROM_SYMBOL_RE.sub("", symbol)
        quantity = _convert_int(row["Quantity"])


Thanks! Made that change here as well.

bayesianmind · 2021-02-14T04:58:12Z

@jbms This is ready to merge alongside #96 when you get a chance. Thanks!

bayesianmind mentioned this pull request Jan 20, 2021

Some work left to do on the Schwab importer #88

Open

carljm requested changes Jan 20, 2021

View reviewed changes

carljm reviewed Jan 20, 2021

View reviewed changes

beancount_import/source/schwab_csv.py Show resolved Hide resolved

carljm reviewed Jan 20, 2021

View reviewed changes

beancount_import/source/schwab_csv.py Outdated Show resolved Hide resolved

bayesianmind marked this pull request as draft February 4, 2021 04:53

zhou13 reviewed Feb 7, 2021

View reviewed changes

bayesianmind force-pushed the schwab_bank branch from 73d7e25 to ca95966 Compare February 14, 2021 04:47

bayesianmind added 5 commits February 13, 2021 20:48

Add Schwab Bank support to Schwab CSV import.

38aa406

Also adds 6 additional BrokerageActions found it my Brokerage CSV imports.

Ignore 0 interest entries

dc61eb0

Consider schwab_action for posting_cleared

ea882ce

Add additional explaination about cleared postings

a8cb33d

Address excellent feedback from carljm

8207a37

Now reusing the same types between brokerage and banking imports.

bayesianmind force-pushed the schwab_bank branch from ca95966 to 8207a37 Compare February 14, 2021 04:48

Fix tox regression I introduced in jbms#90

9618b8d

I didn't realize I should be running tox checks and it turns out the type inference engine wasn't able to correctly infer non-nullability previously. Introducing a variable fixes the check.

bayesianmind commented Feb 14, 2021

View reviewed changes

bayesianmind marked this pull request as ready for review February 14, 2021 04:57

Handle missing action for backfilled entries in journal

692d685

jbms merged commit cc31dc7 into jbms:master Feb 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Schwab bank support to schwab_csv importer #91

Add Schwab bank support to schwab_csv importer #91

bayesianmind commented Jan 11, 2021

coveralls commented Jan 11, 2021 •

edited

Zburatorul commented Jan 12, 2021

bayesianmind commented Jan 12, 2021

carljm left a comment

carljm commented Jan 20, 2021

bayesianmind commented Jan 21, 2021

zhou13 Feb 7, 2021

bayesianmind Feb 14, 2021

bayesianmind left a comment

bayesianmind Feb 14, 2021

bayesianmind commented Feb 14, 2021

		@@ -1077,10 +1319,10 @@ def _load_positions_csv(
		symbol = STRIP_FROM_SYMBOL_RE.sub("", symbol)
		quantity = _convert_int(row["Quantity"])

Add Schwab bank support to schwab_csv importer #91

Add Schwab bank support to schwab_csv importer #91

Conversation

bayesianmind commented Jan 11, 2021

coveralls commented Jan 11, 2021 • edited

Pull Request Test Coverage Report for Build 241

💛 - Coveralls

Zburatorul commented Jan 12, 2021

bayesianmind commented Jan 12, 2021

carljm left a comment

Choose a reason for hiding this comment

carljm commented Jan 20, 2021

bayesianmind commented Jan 21, 2021

zhou13 Feb 7, 2021

Choose a reason for hiding this comment

bayesianmind Feb 14, 2021

Choose a reason for hiding this comment

bayesianmind left a comment

Choose a reason for hiding this comment

bayesianmind Feb 14, 2021

Choose a reason for hiding this comment

bayesianmind commented Feb 14, 2021

coveralls commented Jan 11, 2021 •

edited