redstreet/fava_investor

Contribution guideline on separation from fava/bc code

Closed this issue · 5 comments

Hi. Regarding following note from CONTRIBUTING.md:

### Contribution Guidelines

Each module must include a Fava plugin, a Beancount library, and a Beancount based CLI
(command line interface). APIs in `fava_investor/common/{favainvestorapi.py,
beancountinvestorapi.py}` allow for easily developing these three interfaces to the
library. The goal is to keep fava or beancount specific code neatly separated.

These interfaces are not really decoupling things as their methods are returning data stuctures and classes directly from beancount and fava. If things are going to change there, all code using these interfaces will break anyway.

Looks like we're going to rely on beancount's features a lot and I don't really see a way to avoid coupling. Whats the plan here with these APIs?

Yes, that's the idea: the fava_investor modules themselves should be agnostic to whether they are being called with beancount or fava underneath. The APIs abstract this out for them. This way, they can be called through a CLI without fava, or through fava, and they'll happily work with either. They'll never know if the API layer is using beancount or fava underneath. Does this explanation make sense?

(Note: beancountinvestorapi.root_tree() is the only hack I've temporarily put in that goes against this. I'll remove it once I figure out assetalloc_account which is a WIP.)

I don't understand what value it provides.
Our modules will depend of fava and/or beancount anyway as APIs' methods are returning data structures defined in beancount and fava. There is still coupling there. Logic in our modules (that is processing entries in journal for example) is still relying on beancount's Transaction, Posting, addition of Inventories, etc. That coupling will break them if beancount's code is changed in the future and API classes doesn't seem to help with that.

How would you rewrite libtlh.py to simultaneously work as a) a library without fava dependency (so you can call it from a CLI), and b) usable with a fava context?

BTW, insulation from fava/beancount code is not the goal. We will depend on them heavily and always. The goal is agnosticism to one vs. the other.

Ah okay, got your point now.

Cool. Also: I'm okay moving forward with code that is fava-specific, deferring the goal of making it work with beancount as well. As an example, your balance module uses a fava-specific tree structure which makes it a bit difficult to get it to work without fava. I'm okay with this for now. There is a reason this is called "fava_investor" - fava is primary compared to the library/CLI targets.

Bottomline: please don't worry about the library/CLI aspect of your module for now.