search

beancount / smart_importer

Augment Beancount importers with machine learning functionality.
MIT License
246 stars 28 forks source link

Expose ImporterHook interface #105

Closed bratekarate closed 1 year ago

bratekarate commented 3 years ago

I just created the project beancount-categorizer inspired by smart importer. I'm basically using the same function and interface to apply the hooks and it would be great to get rid of the duplicate code. But since the, ImporterHook Interface ist not exposed by this package, I cannot use it. That's why I propose to expose the interface in this package.

johannesjh commented 3 years ago

Hi, nice project of yours, thank you for pointing it out!

Technically, what is stopping your from importing the ImporterHook like this? 

from smart_importer.hooks import ImporterHook

On a side note, we've had a somewhat related discussion about re-using the hook mechanisms in smart-importer and beancount, compare #97.

Maybe your question is less about the technical possibility of doing the import than about how the two projects can play together? The projects have very similar features and codebases. Would you want to add and maintain the strict categorization as an additional feature of smart_importer?... that could simplify things and it would, in my opinion, make a welcome contribution. What are your further plans for beancount-categorizer?

@yagebu and @tarioch who are co-maintaining the smart_importer package, what do you think?

bratekarate commented 3 years ago

Hi, thanks for the quick reply. Honestly, I did not know it is possible to import it like this. Kind of a noob when it comes to python import mechanisms :).

You are right, the question is generally how the projects interact. Depending on smart_importer is less than ideal, as mentioned in #97. But it would be fine for me at the moment.

While I would be happy to contribute my project to smart_importer, I am not sure this generally solves the underlying issue. Users may want to write their own custom hooks without using smart_importer or developers want to create hook packages for others to use. The ability to add hooks from multiple packages without depending on the apply_hooks function from a specific importer hook package would make the whole ecosystem more pluggable.

I must admit that I did not fully grasp https://github.com/beancount/beancount/commit/ba7b4699d136ab02859013331d98b3c5971c841f yet and have to examine it closer. From the first glance I agree with https://github.com/beancount/beancount/issues/458#issuecomment-638596033 that the hook mechanism of smart_importer seems more flexible and appropriate for my use cases. Also it seems that the smart_importer hooks are not compatible with the built-in beancount hook interface. This means to use smart_importer hooks and regular beancount hooks I have to mix apply_hooks with hooks?

I will think about the best way to go from here. Thanks for the input for now.

EDIT

Ok, I tried to use the hooks argument of scripts_utils.ingest(), but this is obviously not the way to go. As stated in https://github.com/beancount/fava/issues/1184#issuecomment-731831291, this does not work in fava, which is one of my main goals. My idea was to offer one hook for the use with scripts_utils.ingest() and one hook for apply_hooks, so that the user can decide which to use. As this does not seem to be possible, I will stick to cater for apply_hooks only.

I also decided to accept the offer of merging my categorizer into smart_importer if maintainers agree. Nonetheless I strongly support the idea of addding apply_hooks and ImporterHook to the beancount python library so anyone can use it independently, but that's another issue.

johannesjh commented 2 years ago

Do you see a need to merge the two projects? In my opinion, the two projects could happily co-exist without merging. In fact, beancount-categorizer's README explains precisely how to use the two projects together. I suggest the two projects could link to each other in their README files, but I don't quite see the need to merge them into a single code base.

Regarding beancount's import API: I would hesitate to invest into beancount v2's API (this is the beancount version that we are currently using) since Martin is working on a v3 rewrite. I did not look into v3's import API yet, I don't know if it will bring about big changes. One thing that I would personally like to see is a unification of how prices vs. transactions are imported, but that's just my personal two cents.

johannesjh commented 1 year ago

hi, long time no hear... shall we close this issue?