beancount / smart_importer

Augment Beancount importers with machine learning functionality.
MIT License
248 stars 29 forks source link

Unable to use smart_importer with beangulp Importers #117

Closed harinath closed 2 years ago

harinath commented 2 years ago

I get an exception like this:

--8<-- Exception in importer code. Traceback (most recent call last): File "/home/harinath/private/accounts/tools/pypackages/3.10/lib/beangulp/init.py", line 86, in _extract entries = extract.extract_from_file(importer, filename, existing_entries) File "/home/harinath/private/accounts/tools/pypackages/3.10/lib/beangulp/extract.py", line 39, in extract_from_file entries = importer.extract(filename, existing_entries) File "/home/harinath/private/accounts/tools/pypackages/3.10/lib/smart_importer/hooks.py", line 40, in patched_extract_method imported_entries = unpatched_extract( TypeError: Adapter.extract() got an unexpected keyword argument 'existing_entries' --8<--

It appears that the second argument in beangulp.Importer is 'existing', not 'existing_entries'.

harinath commented 2 years ago

Anyway, that's not the only issue. predictor.py uses importer.file_account(). I think the new beangulp.Importer calls that method importer.account().

I ended up using an adapter class that worked well enough:

` class SmartHookAdaptor(Importer): """Adapter to inject SmartHooks to Importer."""

def __init__(self, importer, hooks):
    assert isinstance(importer, Importer)
    self.importer = importer
    self.hooks = hooks

@property
def name(self):
    return self.importer.name

def identify(self, filepath):
    return self.importer.identify(filepath)

def account(self, filepath):
    return self.importer.account(filepath)

def date(self, filepath):
    return self.importer.date(filepath)

def filename(self, filepath):
    return self.importer.filename(filepath)

def extract(self, filepath, existing):
    class FakeImporter:
        """Shim for smart_importer.ImporterHook."""
        def __init__(self, account):
            self.account = account
        def file_account(self, _filepath):
            return self.account

    imported_entries = self.importer.extract(filepath, existing)

    for hook in self.hooks:
        imported_entries = hook(
            FakeImporter(self.account(filepath)),
            filepath, imported_entries, existing)

    return imported_entries

`

johannesjh commented 2 years ago

hi, thank you for reporting this issue and for sharing your workaround.

The smart_importer project still uses beancount v2's apis. So incompatibilities with the new v3 version of beancount and its new importer framework beangulp are to be expected.

some options for how to proceed with this ticket.

harinath commented 2 years ago

My understanding is that beangulp supports beancount v2. So, situations like mine (beangulp with beancount v2) might become more frequent.

I will try to see if I can make my changes less hacky, ... but, as you say, there might need to be more of a rewrite than just sticking in "FakeImporter"s till it works.

johannesjh commented 2 years ago

My understanding is that beangulp supports beancount v2

Are you sure that beangulp targets beancount v2?

beangulp's readme is not very clear about this, but it does say

It should work with the latest version of Beancount: v3. [...] If you're looking for something stable, use the beancount.ingest framework from Beancount branch v2.

...which suggests to me that beangulp is new, unstable, and meant for beancount_v3.

harinath commented 2 years ago

My understanding is that beangulp supports beancount v2

Are you sure that beangulp targets beancount v2?

According to the current maintainer of beangulp (Daniele Nicolodi), it didn't (on Sep 8)

https://groups.google.com/g/beancount/c/yaMv6kdS_7k/m/yY80UNy9BQAJ

but, soon after (on Sep 9), support was added:

https://groups.google.com/g/beancount/c/yaMv6kdS_7k/m/6xdqWDubIAAJ

beangulp's readme is not very clear about this, but it does say

It should work with the latest version of Beancount: v3. [...] If you're looking for something stable, use the beancount.ingest framework from Beancount branch v2.

...which suggests to me that beangulp is new, unstable, and meant for beancount_v3.

Fair enough :-) I'll stick to my local hack, and worry about this later.

johannesjh commented 2 years ago

Alright. I'll close this ticket for now.

Thanks again for bringing it up. Future work on compatibility with beancount v3 is welcome, as well as on compatibility with beangulp. Not sure about the v3 schedule... But it would definitely make sense to work on this once we approach the first official v3 release.

erpreciso commented 4 months ago

Following this announcement, v3 and beangulp are now the main branch in beancount. I adapted my importers to use beangulp, and patched smart_importer to work with beangulp as well. See my patches:

tarioch commented 4 months ago

Thank you very much for this. Right now I'm waiting for Fava to update to V3 before then adjusting smart importer and my own importers as I'm using fava for this.