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

Towards making smart-importer and beancount-import more cooperative. #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Zburatorul
Copy link
Collaborator

Introduced two new methods similar to those present in training.py and new logic.

beancount-import will now look for new_account and unconfirmed_account meta fields in each posting, and understand their presence to mean that the account names of these postings are tentative, potentially new. It will then work in a passthrough mode, offering them for import/modifications.

I don't expect this to actually be merged, but rather to serve as a first draft implementation of the proposal from #23.

@coveralls
Copy link

coveralls commented Jul 27, 2020

Pull Request Test Coverage Report for Build 227

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-68.9%) to 0.0%

Totals Coverage Status
Change from base Build 225: -68.9%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@Zburatorul Zburatorul force-pushed the pre-predicted-postings branch 3 times, most recently from 031c7db to 8f8fcd9 Compare July 28, 2020 04:20
@jbms
Copy link
Owner

jbms commented Jul 28, 2020

Thanks for working on this! It looks like you are on the right track. When you have something working, can you add a test?

@dumbPy
Copy link
Contributor

dumbPy commented Jul 29, 2020

smart-importer wraps the ImporterProtocol.extract and modifies the returned imported_entries.
existing_entries and imported_entries are List[Transaction]. Importers generally write the source posting, and target posting is predicted by smart-importer. deduplication is optional and I see that beancount-import does it well, specially from manually entered transactions and I think this would be a better way to go about it.

We will just need to write a generic ImporterSource(Source) subclass that uses user defined, beancount.ingest.importer.ImporterProtocol importers from the config (possibly wrapped in smart-importer), and if there's a single posting, add Expense:FIXME. Might need more things, but this is all I could think of yet. Will try something if and when I get time.

@dumbPy dumbPy mentioned this pull request Aug 2, 2020
@Zburatorul
Copy link
Collaborator Author

@dumbPy, I had a look at the code in #62 and I think I'm missing the logic of how it fits in with the program of moving the prediction to smart_importer and using beancount-import for the UI. Are the extra meta fields indicating a tentative account meant to be inserted by something upstream of the Source defined in #62?

@dumbPy
Copy link
Contributor

dumbPy commented Aug 7, 2020

The ImporterSource balances the amounts by adding FIXME account that beancount_import may make a prediction for and the UI allows you to modify it. If you wrap the ImporterProtocol subclass in smart_importer's PredictPostings hook, it would predict this 2nd posting and the amount would be anyhow balanced. So now you have got 2 postings, 1 source posting with source_desc and date metas and one predicted by smart_importer. This predicted posting is passed through without beancount-import's prediction (since it's not a FIXME posting), but I am hoping this/some other PR will make it editable in frontend.

@dumbPy
Copy link
Contributor

dumbPy commented Aug 7, 2020

I suppose the UNCONFIRMED_ACCOUNT_KEY is the missing part in this jigsaw. You may add to ImporterSource a func that would add this to any non-source and non-FIXME posting that it may receive from the importer, subject to a bool argument in __init__.

@Zburatorul
Copy link
Collaborator Author

That makes sense. Thanks for the explanation. I'll try weaving it into the code.

dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Dec 22, 2020
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Dec 22, 2020
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Dec 23, 2020
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Dec 23, 2020
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Dec 23, 2020
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
@Zburatorul Zburatorul force-pushed the pre-predicted-postings branch 2 times, most recently from c1b8eb1 to 5d706ee Compare December 29, 2020 21:08
beancount-import will now look for new_account and unconfirmed_account
meta fields in each posting, and understand their presence to mean that
the account names of these postings are tentative, potentially new.

Introduced two new methods similar to those present in training.py
and new logic.
dumbPy added a commit to dumbPy/beancount-import that referenced this pull request Jan 15, 2024
If a posting was predicted by smart_importer or was entered by hand, it should be editable.
This was suggested and discussed in jbms#60
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

Successfully merging this pull request may close these issues.

None yet

4 participants