Skip to content
This repository has been archived by the owner on Mar 1, 2021. It is now read-only.

Port to Beancount #35

Merged
merged 37 commits into from
Feb 3, 2017
Merged

Port to Beancount #35

merged 37 commits into from
Feb 3, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Dec 21, 2016

Reticketing from #28 (comment). Beancount is a plain-text accounting program, derived from Ledger but better-designed, and written in Python 3. 💃 🐍 Beancount comes with a built-in web interface with standard accounting reports, with a third-party interface besides. Let's use Beancount! :D

Todo

  • port files to beancount syntax
  • add bean-check to test suite
  • clean up errors per bean-check
  • port existing test suite
  • clean up bin dir
  • sort out filesystem layout
  • we used to have a separate income statement for escrow—now what?
  • grok cc562c6 and ce1389e
  • think through file structure a bit more
  • go over the README carefully
  • adapt to Beancount's automatic clearing: Port to Beancount #35 (comment)
    • revise 2012-06.beancount
    • figure out escrow reporting
  • trim up list of accounts
  • make sure we're using payee and narration properly
  • sort out Stripe postings
  • give the README one more review

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Dec 21, 2016

Gosh this is so perfect! It's exactly what we were looking for! Great find, @nobodxbodon! 💃

!m @nobodxbodon

screen shot 2016-12-21 at 4 16 05 pm

@chadwhitacre
Copy link
Contributor Author

It shows us errors and source data, too!

screen shot 2016-12-21 at 4 17 34 pm

screen shot 2016-12-21 at 4 32 35 pm

@chadwhitacre
Copy link
Contributor Author

@nobodxbodon Does the todo in the ticket description give you enough to go on here? Do you agree with this direction?

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Dec 21, 2016

Basically I think we should port the existing master to Beancount—get it working with just the 2012-06.beancount file, where "working" means:

  • The file passes bean-check (we should add that check to our test suite for Travis).
  • The file passes our special-purpose tests in bin/test.py.
    • We can do away with any tests in there (if any) that check things that Beancount already guarantees (perhaps test_{income,expenses}_balance?).
  • The file results in the proper reports via bean-web.

Once we are ported over, we can work on deploying a production instance of bean-web or fava in a separate PR. Sound like a plan, @nobodxbodon?

@nobodxbodon
Copy link
Contributor

To make sure, shall I work on this newly created beancount branch?

Also, how about we not delete the original .dat file till .beancount file passes bean-check? Otherwise todo sounds good.

@nobodxbodon
Copy link
Contributor

  • The file passes bean-check (we should add that check to our test suite for Travis).

Done

  • The file passes our special-purpose tests in bin/test.py.
    We can do away with any tests in there (if any) that check things that Beancount already guarantees (perhaps test_{income,expenses}_balance?).

I'll add it after we validate the new report. It seems different from the original output from command line, like #28 (comment)

The original dat file is recovered to pass original test, for now.

  • The file results in the proper reports via bean-web.

Done.

@nobodxbodon
Copy link
Contributor

According to Choosing an Account Type, the existing transactions seem to have some duplicates, like:

; Gittip Payday 0
2012-06-01 * "Users"
Assets:Escrow:Samurai 2.96 USD
Assets:Fee-Buffer:Samurai 0.34 USD
Income:Fee-Buffer:Samurai -0.34 USD
Income:Escrow:Samurai -2.96 USD
2012-06-01 * "Balance Sheet"
Income:Escrow:Samurai 2.96 USD
Income:Fee-Buffer:Samurai 0.34 USD
Liabilities:Fee-Buffer -0.34 USD
Liabilities:Escrow -2.96 USD

This leads to empty (all 0) incoming statement in beancount. Trying to merge them.

@nobodxbodon
Copy link
Contributor

@whit537 @kaguillera May I check the meaning of the terms:

  • "Escrow": money received by receiver
  • "Fee-Buffer": money paid by giver
  • "Operations"?

And I suppose Samurai, New-Alliance, IHasAMoney, Stripe, VerificationAndTesting are all receivers?

What's Escrow:Cash?

@chadwhitacre
Copy link
Contributor Author

@nobodxbodon Have you read through "How Our Books are Organized"? We tried to explain the terms there. If it's unclear let's figure out how to upgrade that doc.

@@ -3,6 +3,38 @@
option "title" "Gratipay Finances"
option "operating_currency" "USD" ; The main currencies you use

** Accounts ; Open all the accounts you have
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we want to use these accounts across all month files for the fiscal year, we should store them in a separate file. Under Ledger we were using FY2013.dat and declarations.dat. We will need to figure out a) what files it makes sense to use for Beancount, and b) how to make Beancount look at multiple files at once (simple cat?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chadwhitacre
Copy link
Contributor Author

To make sure, shall I work on this newly created beancount branch?

Yes! :-)

Also, how about we not delete the original .dat file till .beancount file passes bean-check?

Suit yourself. I figured since it was still on master we didn't need it here. :-)

It seems different from the original output from command line, like #28 (comment)

Howso?

@nobodxbodon
Copy link
Contributor

nobodxbodon commented Dec 28, 2016

Have you read through "How Our Books are Organized"? We tried to explain the terms there. If it's unclear let's figure out how to upgrade that doc.

Ah that clears much. Sorry I overlooked. I'll need to check each transaction to understand better.

Howso?

The numbers don't match with the original report from command line. Like the Equity:Owners:Chad part.

------ Update -----
I just realize I was using 2012-06 instead of FY2013. After including 2012-06.beancount from FY2013.beancount, Assets total and equity from you are consistent with original report. Still need to fix liability part. Commits coming later.

@chadwhitacre
Copy link
Contributor Author

According to Choosing an Account Type, the existing transactions seem to have some duplicates, like:

I don't believe those are true duplicates. We did it that way to get the income statement to come out properly under Ledger. Open to revising if need be.

@nobodxbodon
Copy link
Contributor

nobodxbodon commented Dec 29, 2016

I got the numbers agree with original reports. As I understand from the source code of original balance-sheet and income-statement, they both read only part of the ledger file, and the parts titled with 'balance sheet' seem to be part of corresponding transaction. So they can be merged to transaction, as in latest commit?

Still I feel uncertain about the accounts I created, like: Liabilities:Fee-Buffer:Samurai

screen shot 2016-12-28 at 11 26 02 pm
screen shot 2016-12-28 at 11 26 14 pm

@nobodxbodon
Copy link
Contributor

nobodxbodon commented Dec 29, 2016

Here's the first transaction in original dat. Could you check if I understand the items correctly?:

2012-06-01  Users
    Assets:Escrow:Samurai            $     2.96 <-- amount given to user Samuri
    Assets:Fee Buffer:Samurai        $     0.34 <-- processing fee of $2.96 above. Why isn't it negative?
    Income:Fee Buffer:Samurai       -$     0.34
    Income:Escrow:Samurai           -$     2.96
2012-06-01  Balance Sheet
    Income:Escrow:Samurai            $     2.96
    Income:Fee Buffer:Samurai        $     0.34 <-- above 4 lines sum up to 0, so I removed them all in beancount
    Liabilities:Fee Buffer          -$     0.34
    Liabilities:Escrow              -$     2.96

Besides, why is there a $31.35 fee buffer here? It seems much larger than normal fee:

2012-06-04  Samurai
    Expenses:Fee Buffer:Samurai      $    31.35
    Assets:Fee Buffer:New Alliance  -$    31.35

@chadwhitacre
Copy link
Contributor Author

@nobodxbodon I've taken the liberty of reformatting your comment for better readability, using triple-backticks.

@chadwhitacre
Copy link
Contributor Author

amount given to user Samurai

No, Samurai was our first payment processor. Individual users don't show up on our books here, only in the Gratipay database. The reconciliation dashboard is the intended bridge between the two. So the 2.96 here is the amount by which our escrow-held-at-Samurai increased during this transaction.

processing fee of $2.96 above. Why isn't it negative?

Different processor handle fees different ways. I will have to go back through to get it all straight, but basically some of them send us all the money first and then draw some of it back, whereas others only send us the net. Either way, though, the fee is coming out of the money that was charged (in aggregate here; we don't see individual charges as mentioned above). So basically there is an aggregate settlement into Samurai of $3.30 for this day, and $0.34 of that is fees. We account for that using the Fee Buffer for reasons described in the README.

above 4 lines sum up to 0, so I removed them all in beancount

Those lines were added so that the transaction would show up on the income statement. What is the effect of removing those lines on beancount's income statement? I haven't checked yet but that is the question I have.

Besides, why is there a $31.35 fee buffer here? It seems much larger than normal fee:

Yeah, Samurai was sloppy with their own accounting and made several errors. @kaguillera and I went through this with a fine-toothed comb vis-a-vis our bank statement when we initially produced the file; I'm sorry if we didn't comment it well enough. I will try to revisit ...

@chadwhitacre
Copy link
Contributor Author

Off the top of my head:

Payment Processors: Samurai, Cash, Stripe, Balanced, PayPal, Coinbase, Braintree, TransferWise
Banks (Escrow): New Alliance, Ally, Citizens
Banks (Operations): New Alliance, Ally, Citizens, PNC

italics = current

@nobodxbodon
Copy link
Contributor

No, Samurai was our first payment processor. Individual users don't show up on our books here, only in the Gratipay database.

Oh thanks for this and the explanation of fee buffer. Now it makes much more sense for me. After re-reading the README I think it already implicitly indicates those are processors, but I didn't realize.

What is the effect of removing those lines on beancount's income statement? I haven't checked yet but that is the question I have.

I don't think those sum up to 0 affect income statement. I got the same income statement after removing these lines in 2012-06.dat:

    Income:Fee Buffer:Samurai                                      -$     0.34
    Income:Escrow:Samurai                                          -$     2.96
2012-06-01  Balance Sheet
    Income:Escrow:Samurai                                           $     2.96
    Income:Fee Buffer:Samurai                                       $     0.34

Besides, as shown in the screenshot I posted, Beancount outputs the same income statement.

I will try to revisit ...

That will be ideal, but maybe we can start with the latest first, and backfill/revisit when there's bandwidth.

@nobodxbodon
Copy link
Contributor

How can we do the same as 'bean-check' programatically in test.py?

@chadwhitacre
Copy link
Contributor Author

How can we do the same as 'bean-check' programatically in test.py?

@nobodxbodon Can we shell out to the bean-check CLI?

@nobodxbodon
Copy link
Contributor

nobodxbodon commented Dec 30, 2016

@whit537 you mean using subprocess? I suppose so, but would it work with Travis? Can Travis call bean-check?

BTW how early you get up!

@chadwhitacre
Copy link
Contributor Author

Up earlier than usual this morning. I'll probably crash again in a bit. How late you stay up! :-)

We're already calling ledger via subprocess or equivalent, aren't we?

@chadwhitacre
Copy link
Contributor Author

This is looking great, @nobodxbodon! I made a couple commits to clean up the file structure—include is a great find!—and bring us into full parity with the balance sheet and income statement on master. With 368346d I think we're there! 💃

Old

screen shot 2016-12-30 at 9 35 18 am

New

screen shot 2016-12-30 at 9 35 54 am
screen shot 2016-12-30 at 9 35 34 am

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Dec 30, 2016

@nobodxbodon Did you get anywhere last night with calling bean-check from bin/test.py?

@nobodxbodon
Copy link
Contributor

Did you get anywhere last night with calling bean-check from bin/test.py?

@whit537 pitifully not really. The question was my last activity :)

@chadwhitacre
Copy link
Contributor Author

At least that means I didn't clobber any of your work by pushing commits overtop of yours. :)

@nobodxbodon
Copy link
Contributor

About porting existing test suite, here are some tests covered already by bean-check. Maybe there are more:

test_income_balances
test_expenses_balance
test_net_income_reconciles_with_current_activity  // because current equity is calculated by income+expense already

The ones left:

test_escrow_balances
test_fee_buffer_balances
test_fee_buffer_income_reconciles_with_fee_buffer_liability

@chadwhitacre
Copy link
Contributor Author

If it's an income/expense it should just be "processing fees," not "fee buffer." But ... what's the problem with tracking it as a fee buffer the whole way back to the beginning?

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Feb 2, 2017

I guess that's just not accurate. How will we institute the fee buffer in FY2014, though? I guess we can cross that bridge when we come to it?

@chadwhitacre
Copy link
Contributor Author

I made some commits, but 621eba0 is actually buggy. More work required ...

@chadwhitacre
Copy link
Contributor Author

... but I'm out of time for now. I'll be back.

It wasn't implemented until June, 2013, as now noted in the README.
@chadwhitacre
Copy link
Contributor Author

Alright, fee buffer removal redone in 150c3d0. You good with that, @nobodxbodon et al.?

@chadwhitacre
Copy link
Contributor Author

I can see in the screenshot that Stripe held all together $25.29 before 2/28, and $20.97 was transfered to new alliance on 2/28, with $4.32 fee. But how can I tell that $20.67 in this $20.97 was escrow?

That info comes from the Gratipay database, per fee and amount in exchanges. I believe we had a detailed view for admins on the reconciliation dashboard that gave this, but maybe that is gone now? I'll have to check ...

BTW in the log, I don't even see the $25.29 in either New Alliance or Stripe statement. Do we have another source of statements?

Um ... the 25.29 is in that Stripe screenshot, no? They clear us the net (not the gross, as Samurai did), so we wouldn't expect to see 25.29 in New Alliance. You want to see 25.29 somewhere in the logs repo, is that it?

@chadwhitacre
Copy link
Contributor Author

You want to see 25.29 somewhere in the logs repo, is that it?

Looks like the CSVs we got from Stripe show the net they paid, not the gross or fees, e.g.:

https://github.com/gratipay/logs/blob/master/statements/2012/06/stripe.csv

@chadwhitacre
Copy link
Contributor Author

I'll have to check ...

Yeah, looks like that is gone. I don't think we should block this on that, though, since we're not fundamentally changing the numbers here.

@chadwhitacre
Copy link
Contributor Author

Last thing from my pov here is to give the README and data file one more review.

@chadwhitacre
Copy link
Contributor Author

README looks fine to me, taking a last pass through the beancount file ...

@chadwhitacre
Copy link
Contributor Author

Let's go ahead and record a single transaction for the Stripe transfer (2b7320e), since that's how the info is coming to us in statements as at #35 (comment).

@chadwhitacre
Copy link
Contributor Author

screen shot 2017-02-03 at 11 18 37 am

screen shot 2017-02-03 at 11 18 51 am

@chadwhitacre
Copy link
Contributor Author

I've confirmed that the sum of Assets:{Escrow,Operations}:New-Alliance matches the balance in our bank statement for that month.

@chadwhitacre
Copy link
Contributor Author

screen shot 2017-02-03 at 11 25 20 am
screen shot 2017-02-03 at 11 25 45 am

@chadwhitacre
Copy link
Contributor Author

Okay, the balance sheet as of e6c9998 matches the balance sheet in #35 (comment), with the fee buffer (28.86) folded into current activity, and no final Stripe expense (14.63).

-9.79 + 28.86 - 14.63 = 4.44

@nobodxbodon I'm ready for a merge. How about you? :)

@nobodxbodon
Copy link
Contributor

LGTM! Shall I merge?

@mattbk
Copy link

mattbk commented Feb 3, 2017

Meeeeeerge...

@chadwhitacre
Copy link
Contributor Author

@nobodxbodon I'm ready for you to merge! :-)

@nobodxbodon nobodxbodon merged commit 50d526a into master Feb 3, 2017
@nobodxbodon nobodxbodon deleted the beancount branch February 3, 2017 19:43
@chadwhitacre
Copy link
Contributor Author

Woo-hoo! Awesome work, everyone! Gratipay is now officially on Beancount! 💃 🌻

@chadwhitacre
Copy link
Contributor Author

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants