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

Don't consume all provided arguments in new ingestion flow #12

Open
blais opened this issue Jan 9, 2019 · 4 comments
Open

Don't consume all provided arguments in new ingestion flow #12

blais opened this issue Jan 9, 2019 · 4 comments

Comments

@blais
Copy link
Member

blais commented Jan 9, 2019

Original report by Zahan Malkani (Bitbucket: Zahan, GitHub: zahanm).


An issue with the implementation in
https://bitbucket.org/blais/beancount/commits/0c66d90c2173fd5fcc83d10c8297800d70a81c9b
which itself addresses the questions raised in beancount/beancount#75
is this:

you can't provide custom arguments to your own import script, because once you call ingest(..) - it is unhappy that it doesn't recognize some of the arguments.

A simple fix would be to parse_known_args instead, which I'll do in a PR shortly - once I figure out how to make a PR on bitbucket.

@blais blais transferred this issue from beancount/beancount Feb 2, 2021
@dnicolodi
Copy link
Collaborator

@zahanm Can you please elaborate on how you were planning to use additional command line arguments to the ingest script? Beancount ingest framework has been spun off to this separate repo and the command line interface redesigned using Click. I would like to enable your use case in the new code, if it does not deviate too much from the design.

@zahanm
Copy link

zahanm commented Feb 14, 2021

Hi @dnicolodi - thanks for reaching out.
This was a while ago, but I believe I was trying to pass in a custom argument, namely the location of a configuration file, that I would then use in the rest of my script.

I ended up rearchitecting things, and doing most of it by hand. Though I'd be happy to use a new API if it supports a richer ingestion API.

  1. A step to download the transactions, and persist them in a "shelf" file. https://github.com/zahanm/collect-beans/blob/master/collector.py
  2. A step that takes those transactions and inserts them in the right place in my journal file. https://github.com/zahanm/collect-beans/blob/master/insert.py

@zahanm
Copy link

zahanm commented Feb 14, 2021

https://github.com/zahanm/collect-beans/blob/8591b7da51f6036df144e1f98626058bf9c26fb9/collector.py#L417

Yeah, this is the argument in particular I wanted to pass in.

@dnicolodi
Copy link
Collaborator

dnicolodi commented Feb 14, 2021

Adding commands to the script entry point generated by the ingestion framework is not possible. There is an example in the comments to the PR that implemented it #28, although it may require some adjustments as the API may ended up to be slightly different than the one I had in mind when writing the comment.

On the other hand, adding options or arguments to the script entry point generated by the ingestion framework or to the existing commands is not possible because this would also need to add code that does something with the extra arguments. What you can do, however, is to subclass the new Ingest class https://github.com/beancount/beangulp/blob/master/beangulp/__init__.py#L88 in a way similar to

class Ingest(beangulp.Ingest):
    def __init__(self, importers, hooks=None):
        self.importers = importers
        self.hooks = hooks

        @click.group()
        @click.option('--config', ...)
        @click.version_option()
        @click.pass_context
        def main(ctx, config):
            """Import data from and file away documents from financial institutions."""
            ctx.obj = self
            ctx.obj.config = config

        main.add_command(beangulp._extract)
        main.add_command(beangulp._file)
        main.add_command(beangulp._identify)

        self.main = main

this is completely untested, but I think you get the spirit. Things could be reorganized a bit to avoid having to copy the whole __init__ of the parent class in the subclassed one. If you think that this is useful, I may look into it.

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

3 participants