beancount / smart_importer

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

Remove suggestions because suggest=True adds list as metadata which is invalid #91

Closed tbm closed 3 years ago

tbm commented 4 years ago

When you set suggest=True, a list is added as __suggested_xxx__ metadata but lists are not valid as beancount metadata. How is suggest=True supposed to be used? I assume a script must be used to get rid of the metadata and set the payee before it's passed to beancount but this isn't documented anywhere.

smart_importer/predictor.py:

    def apply_suggestion(self, entry, suggestions: List[str]):
...
            key = "__suggested_{}s__".format(self.attribute)
            entry.meta[key] = suggestions
  File "/home/tbm/.local/lib/python3.7/site-packages/beancount/parser/printer.py", line 145, in write_metadata
    raise ValueError("Unexpected value: '{!r}'".format(value))
yagebu commented 4 years ago

This intended to be used in plugins/other importers/as Fava input suggestions (https://github.com/beancount/fava/issues/801, still TODO).

tbm commented 4 years ago

Would it make sense to propose a list of strings to be allowed for metadata in beancount?

(I'd actually have use cases for that.)

yagebu commented 4 years ago

Would it make sense to propose a list of strings to be allowed for metadata in beancount?

I'd be against that - another case to deal with in all places in Fava and Beancount where metadata is handled. Also, for any specific use case a quick "encode" as one space-separated string is probably enough - and if it isn't, maybe you're trying to stuff something to complicated into the metadata (and encoding to JSON should do it in that case)

tbm commented 4 years ago

You're making some good points.

Anyway, please leave this issue open as a documentation issue for smart_importer.

johannesjh commented 3 years ago

I closed beancount/fava#801 because I don't really see the need for it. Fava's existing suggestions work sufficiently well in my experience.

Shall we remove smart_importer's suggestion feature?

yagebu commented 3 years ago

Shall we remove smart_importer's suggestion feature?

Yep, sounds good to me

tarioch commented 3 years ago

Agree as well, for me works perfectly either the defaulting from smart importer works (which is in about 95% of my cases) or otherwise it's easy to adjust to what I need.