Add ofx support for treasury T-Bill #187

zhou13 · 2023-02-06T07:46:26Z

Add ofx support for treasury T-Bill
Workaround the issues that Fidelity OFX shows the units of treasuries 100x larger.

Zburatorul

@zhou13, thank you for identifying a real gap in ofx.py and closing it.

Zburatorul · 2023-02-06T18:04:06Z

beancount_import/source/ofx.py

@@ -785,6 +786,10 @@ def get_security(unique_id: str) -> Optional[str]:
                return None
            sec = securities_map[unique_id]
            ticker = sec.ticker
+            # Treasury bill and bond start with 912
+            if ticker.startswith("912"):
+                # Append "T" to make it a valid ticker


Was this meant to be prepend rather than "append"?

Yes, fixed.

Zburatorul · 2023-02-06T18:04:43Z

beancount_import/source/ofx.py

@@ -785,6 +786,10 @@ def get_security(unique_id: str) -> Optional[str]:
                return None
            sec = securities_map[unique_id]
            ticker = sec.ticker
+            # Treasury bill and bond start with 912
+            if ticker.startswith("912"):


Is the ticker of a bond always the same as its CUSIP?

It is at least for Fidelity's Ofx. I don't have other OFX so I am not sure about it.

I can verify this is also true for vanguard treasury.

beancount_import/source/ofx.py

zhou13 · 2023-02-07T06:42:29Z

Pytest runs fine locally, not sure what happens. Also the balance change make a lot of changes to the existing tests.

Zburatorul · 2023-02-07T21:18:27Z

Pytest runs fine locally, not sure what happens. Also the balance change make a lot of changes to the existing tests.

That's exactly what happens. We need to figure out if the changes are correct or not.

Here is the OFX specification that describes the meaning of each field in detail.

For UNITS it says:

For stocks, MFs, other, number of shares held.
Bonds = face value.
Options = number of contracts

Meanwhile for UNITPRICE it saysBonds = percentage of par. Meaning a number from 0 to 100, not a dollar amount.
We could just ignore all this and record UNITPRICE as the price, but it would produce incorrect (about 100X inflated) asset values. This also explains why Fidelity has that 100X factor from your original commit.

There are two ways out: either we modify the amount, or the price.
I'm leaning towards transforming the price to match beancount't expectation of what a price is (market value/quantity of security) rather than modifying the quantity/units.
There's a subcase where we detect that it's a bond, and simply divide the price by 100X. But my suggested code takes care of that AND of situations where the multiplier is not 100.

There are a number of ofx test files where market value != price * units. That is why the tests are failing.
For example testdata/source/ofx/vanguard_roth_ira.ofx has:

  <UNITS>46.872
  <UNITPRICE>84.20
  <MKTVAL>72.85

But it's not difficult to fix them, and it would result in a PR that is more general and more robust, in my opinion.
The work consists of inspecting each failed test and seeing that 1) no balance entries are modified 2) the new price indeed produces the correct market value.

We're really close here. Are you willing to get it over the finish line?

…ty-buydebt

zhou13 · 2023-02-26T02:41:30Z

Do you know how to reproduce the CI error locally? Currently running

pytest -s beancount_import/source/ofx_test.py

on my branch give me no error (only warnings).

Zburatorul · 2023-02-26T02:55:50Z

Maybe you have a local file that that you forgot to check in?

Zburatorul · 2023-02-26T03:01:15Z

Here's a sample error:

E - 2018-07-03 price TYCDT 1.554232804232804232804232804 USD
E + 2018-07-03 price TYCDT 84.20 USD

It indicates that you changed the price, but did not modify the expected output file. Please double-check that the new price is actually correct.

zhou13 · 2023-02-26T05:56:45Z

Here is what I just did:

git clone https://github.com/zhou13/beancount-import bc2
cd bc2
git checkout fidelity-buydebt
pip install git+https://github.com/zhou13/beancount-import@fidelity-buydebt
pytest -s beancount_import/source/ofx_test.py

It passes all the test. The change E - 2018-07-03 price TYCDT 1.554232804232804232804232804 USD is from executing check_source_example(..., write=True) on my local machine. I don't understand why the result is different on the CI.

Zburatorul · 2023-02-26T18:27:30Z

Yeah it is strange. I don't understand why the file in the PR is different from the one in the test.
Try git push --force once again, and let's re-run the test.

If nothing changes, then I suggest you revert the testdata/source/ofx/test_vanguard_roth_ira_matching/import_results.beancount back to master, commit & force push that. We might get a new clue from that test passing/failing in CI.

…ty-buydebt

zhou13 · 2023-04-16T06:41:13Z

@Zburatorul I think this PR is ready to go. Previously I missed a test, which is why it fails. I checked investment_buy_sell_income.ofx and I believe the new result is more likely to be correct as MKTVAL is a more reliable entry when it conflicts with UNITPRICE.

Zburatorul

A few questions

beancount_import/source/ofx.py

testdata/source/ofx/fidelity.ofx

testdata/source/ofx/test_vanguard_roth_ira_matching/training_examples.json

testdata/reconcile/test_ofx_matching/0/candidates.diff

…ty-buydebt

zhou13 · 2023-07-12T06:19:59Z

@Zburatorul I have been using this branch for a while for Fidelity Treasury bill and it works well for me. I also add tolerance to workaround buggy rounding of the unit entry in Vanguard's bond ofx so it can generate reasonable balance. Let me know if you have any additional concerns.

Zburatorul · 2023-07-15T19:35:15Z

Thank you for the contribution @zhou13!

ankurdave · 2023-10-09T20:18:21Z

beancount_import/source/ofx.py

@@ -797,6 +833,10 @@ def get_security(unique_id: str) -> Optional[str]:
                return None
            sec = securities_map[unique_id]
            ticker = sec.ticker
+            # Treasury bill and bond start with 912
+            if ticker.startswith("912"):


FYI, this will crash if the ticker is None, rather than printing a helpful error message as it used to.

zhou13 changed the title ~~Add ofx support for treasure T-Bill~~ Add ofx support for treasury T-Bill Feb 6, 2023

initial commit

def67a9

zhou13 force-pushed the fidelity-buydebt branch from 5bbb45c to def67a9 Compare February 6, 2023 07:51

Zburatorul reviewed Feb 6, 2023

View reviewed changes

zhou13 added 3 commits February 6, 2023 22:16

Address review comments

0eb08cf

Restore ofx_test.py

db39a3a

Update tests

54cfba5

Merge branch 'master' of github.com:jbms/beancount-import into fideli…

c6bbfcb

…ty-buydebt

zhou13 added 8 commits March 11, 2023 22:10

Merge branch 'master' of github.com:jbms/beancount-import into fideli…

4b8fc87

…ty-buydebt

Add sellother

e197749

Update ofx

7bff28b

Bug fix

f387f69

Fix test

589a359

Fix test

2a2cfea

Update test

db9847c

Add comments

db24215

Zburatorul reviewed Apr 16, 2023

View reviewed changes

zhou13 added 5 commits April 16, 2023 16:53

address reviewer comments

df9c3eb

change diviends to interest and add tests

5ea5b47

fix typo

cf0003e

Make it compatible with Vanguard

0169586

Merge branch 'master' of github.com:jbms/beancount-import into fideli…

19e9317

…ty-buydebt

zhou13 force-pushed the fidelity-buydebt branch from e25268f to 19e9317 Compare July 11, 2023 22:02

Zburatorul merged commit e790b64 into jbms:master Jul 15, 2023

zhou13 deleted the fidelity-buydebt branch July 16, 2023 03:13

ankurdave reviewed Oct 9, 2023

View reviewed changes

pallavg mentioned this pull request Dec 12, 2023

Importer doesn't seem to recognize some transactions from QFX from Merrill Edge #217

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ofx support for treasury T-Bill #187

Add ofx support for treasury T-Bill #187

zhou13 commented Feb 6, 2023 •

edited

Loading

Zburatorul left a comment

Zburatorul Feb 6, 2023 •

edited

Loading

zhou13 Feb 7, 2023

Zburatorul Feb 6, 2023

zhou13 Feb 7, 2023

zhou13 Jul 11, 2023

zhou13 commented Feb 7, 2023 •

edited

Loading

Zburatorul commented Feb 7, 2023

zhou13 commented Feb 26, 2023 •

edited

Loading

Zburatorul commented Feb 26, 2023

Zburatorul commented Feb 26, 2023

zhou13 commented Feb 26, 2023 •

edited

Loading

Zburatorul commented Feb 26, 2023

zhou13 commented Apr 16, 2023 •

edited

Loading

Zburatorul left a comment

zhou13 commented Jul 12, 2023

Zburatorul commented Jul 15, 2023

ankurdave Oct 9, 2023

Add ofx support for treasury T-Bill #187

Add ofx support for treasury T-Bill #187

Conversation

zhou13 commented Feb 6, 2023 • edited Loading

Zburatorul left a comment

Choose a reason for hiding this comment

Zburatorul Feb 6, 2023 • edited Loading

Choose a reason for hiding this comment

zhou13 Feb 7, 2023

Choose a reason for hiding this comment

Zburatorul Feb 6, 2023

Choose a reason for hiding this comment

zhou13 Feb 7, 2023

Choose a reason for hiding this comment

zhou13 Jul 11, 2023

Choose a reason for hiding this comment

zhou13 commented Feb 7, 2023 • edited Loading

Zburatorul commented Feb 7, 2023

zhou13 commented Feb 26, 2023 • edited Loading

Zburatorul commented Feb 26, 2023

Zburatorul commented Feb 26, 2023

zhou13 commented Feb 26, 2023 • edited Loading

Zburatorul commented Feb 26, 2023

zhou13 commented Apr 16, 2023 • edited Loading

Zburatorul left a comment

Choose a reason for hiding this comment

zhou13 commented Jul 12, 2023

Zburatorul commented Jul 15, 2023

ankurdave Oct 9, 2023

Choose a reason for hiding this comment

zhou13 commented Feb 6, 2023 •

edited

Loading

Zburatorul Feb 6, 2023 •

edited

Loading

zhou13 commented Feb 7, 2023 •

edited

Loading

zhou13 commented Feb 26, 2023 •

edited

Loading

zhou13 commented Feb 26, 2023 •

edited

Loading

zhou13 commented Apr 16, 2023 •

edited

Loading