I've cleaned up and added some unit tests. The preliminary PR is here: https://github.com/beancount/beancount/pull/840
The main issue right now is that it only works for transfers with two equal and opposite augment/reduce postings (and worse, does not gracefully detect and reject more complex forms, such as a transfer from one account into two others, or from two others into one). The case of split-transferring from one account into two others (e.g., account1: -100 HOOL, account2: 50 HOOL, account3: 50 HOOL) is especially problematic as there's no way to know which cost units should go to account2 and which to account3. Preliminary feedback from anyone would be very much appreciated, specifically: - this question of how to handle and/or reject more-complex-transfers - cases to add more tests for - anything about the code itself On Sat, Apr 13, 2024 at 2:39 PM Eric Altendorf <ericaltend...@gmail.com> wrote: > Awesome, thanks. This sounds good: > 1) I'll add unit tests, try to clean up some of the questions and todos i > had, and develop more certainty around the problem and solution > 2) I'll share with you my findings and the unit tests, and you can try > running it on your data to ensure things match > 3) Then we can make a call on creating a branch or a conditional in the > master branch. > > fwiw i'm not too worried about divergence -- i created this at least 6 > months ago, and just pulled from head with no merge conflicts, so i don't > think this is very active part of the code base :) > > On Sat, Apr 13, 2024 at 6:57 AM Martin Blais <bl...@furius.ca> wrote: > >> - There's no dev list, this is it, not enough traffic to justify a >> separate list. >> - Your feature touches a very sensitive part of the booking process - one >> I've struggled to get sort of right in the past, has a lot of weird >> corner cases, it's been a bit of a whack-a-mole situation - I'd have to >> really immerse myself back into this to give it proper review, and I don't >> have time for a while. >> > The more I look at it, yeah...... I agree. This feels like it is calling out for an implementation in a higher-level, more declarative language more amenable to static analysis and correctness proofs. No idea what that language is, though.... :) - I would suggest adding unit tests for the specific changes it improves >> would be very useful to that effect. >> - In the meantime I'm happy to create for you a permanent branch in the >> beancount repo itself to track this, or perhaps even better, to install a >> conditional in the master branch with an option that dispatches between the >> two implementations. >> - The cheap (time-boxed) and easy thing I could do could be to ensure >> before/after results match. >> LMK what you'd like to do, >> >> >> >> On Thu, Apr 11, 2024 at 12:10 PM Eric Altendorf <ericaltend...@gmail.com> >> wrote: >> >>> I have revisited the work I did earlier to tweak Beancount to propagate >>> cost basis with asset transfers. (As has been discussed, this is a bit of >>> a corner case in general, but is actually very common and important for >>> capital gains calculations for cryptocurrency assets, since they are >>> frequently transferred between accounts.) >>> >>> Digging through my git repo, it looks like what I did was this: >>> >>> https://github.com/beancount/beancount/compare/master...ericaltendorf:beancount:cost-transfer?expand=1 >>> >>> I would appreciate some feedback on whether this generally looks like >>> the right approach, as well as any shortcomings that would need to be >>> addressed before I can send a PR to merge it into Beancount proper. >>> >>> Thanks, >>> Eric >>> >>> (Is there a "beancount-dev" list that would be better to send this to? :) >>> >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Beancount" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to beancount+unsubscr...@googlegroups.com. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/beancount/CAFXPr0u1vHW-5C32d_LvpgWXTd_quhsk-UGP8U%2BO8VU9syj3xw%40mail.gmail.com >>> <https://groups.google.com/d/msgid/beancount/CAFXPr0u1vHW-5C32d_LvpgWXTd_quhsk-UGP8U%2BO8VU9syj3xw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- >> You received this message because you are subscribed to the Google Groups >> "Beancount" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to beancount+unsubscr...@googlegroups.com. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/beancount/CAK21%2BhMuNzXX61TJqRawP3Cq-jK8U1Weokra9rmd19PCMbYBkw%40mail.gmail.com >> <https://groups.google.com/d/msgid/beancount/CAK21%2BhMuNzXX61TJqRawP3Cq-jK8U1Weokra9rmd19PCMbYBkw%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > -- You received this message because you are subscribed to the Google Groups "Beancount" group. To unsubscribe from this group and stop receiving emails from it, send an email to beancount+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CAFXPr0sjamMnTLfduNYn6GykZRRMTw2oFcFrEx7JjfWhz9Tanw%40mail.gmail.com.
