Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All transaction builders should be fully compatible with all readers #116

Open
GalmWing opened this issue Jan 3, 2025 · 3 comments
Open

Comments

@GalmWing
Copy link

GalmWing commented Jan 3, 2025

paycheck.Importer is strongly tied to multitable readers, the dependency on self.alltables is not evident, and it means that certain libreaders are not compatible with it.

In a general sense I think either:

  1. Every transaction builder should be compatible with every libreader
  2. There should be different base reader/importer categories and importers should adhere to their own category to ensure they are fully compatible

For example, for this combo of paycheck.Importer, csvreader.Importer, I must customize the code enough that I lose a lot of the goodness of the framework.

So far I think there are only 2 ways readers organize data:

  1. as a single table/list of values
  2. as multiple tables

Perhaps (2) is not strictly necessary to pass to the transaction builders, it could be converted to a single table. There could be ways to deal with any issue like potential name collisions between tables

@redstreet
Copy link
Owner

Very much agree. The paycheck transaction builder in particular could use some help in conforming. I haven't thought this in a while, but remember thinking a multitable data structure could become the standard output for readers, since single tables are covered by it too. Further design and/or PRs welcome.

@GalmWing
Copy link
Author

GalmWing commented Jan 4, 2025

I'm just getting started with beancount, so likely contributions are not going to be up to par until I have better understanding.
To unblock myself I wedged in a table into the multi table format, it's a hardcoded category, so it's not pretty, but is just a couple of lines of code. Though I did have to peruse the code to figure out there was a raw_rdr

class Importer(paycheck.Importer, csv_multitable_reader.Importer):
    def custom_init(self):
        self.alltables = {}

    def prepare_tables(self):
        self.alltables = {'<category>': self.raw_rdr}

@redstreet
Copy link
Owner

No worries. I'll leave this ticket open as a long-term improvement item until you or someone else can cook up a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants