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

Add UNCONFIRMED to externally predicted postings of imported entries #82

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dumbPy
Copy link
Contributor

@dumbPy dumbPy commented Dec 22, 2020

If a posting was predicted by smart_importer or was entered by hand, it won't be predicted again but should be editable like the FIXME or predicted entry by beancount-import.

This was suggested and discussed in #60 and will start working once #60 is completed and merged.

@coveralls
Copy link

coveralls commented Dec 22, 2020

Pull Request Test Coverage Report for Build 222

  • 29 of 29 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 68.986%

Totals Coverage Status
Change from base Build 216: 0.1%
Covered Lines: 5160
Relevant Lines: 7245

💛 - Coveralls

@jbms
Copy link
Owner

jbms commented Dec 22, 2020

Please change variable name to UNCONFIRMED

@Zburatorul
Copy link
Collaborator

@dumbPy, glad you went ahead with this. How do I set up the minimal example to test that #62 works?

@dumbPy dumbPy force-pushed the add-unconfirmed-account-key branch 2 times, most recently from 226742a to 891d5c7 Compare December 23, 2020 06:00
@dumbPy
Copy link
Contributor Author

dumbPy commented Dec 23, 2020

Edit:

You may use the test_dummy_predictor test added in 50c6ba9 to test the input and working of this PR.

Original Comment (outdated):

@Zburatorul use below code as generic_importer_source_test.py. It will add dummy postings to all the imported transactions just like the smart_importer. the basic_test fails with it, but the other two tests pass.

You can also use the DummyPostingPredictor from below with any other importer and statements you have, and it will add the balanced Dummy posting. #82 adds {"unconfirmed_account":""} to meta of this Dummy Posting. #60 should make this posting editable from frontend.

from decimal import Decimal
import os

import pytest

from .source_test import check_source_example
from beancount.ingest.importers.csv import Importer as CSVImporter, Col
from beancount.core.data import Transaction, Posting, Amount

testdata_dir = os.path.realpath(
    os.path.join(
        os.path.dirname(__file__), '..', '..', 'testdata', 'source', 'generic_importer'))

testdata_csv = os.path.join(testdata_dir, "csv")

examples = [
    'test_basic',
    'test_invalid',
    'test_training_examples'
]

importer = CSVImporter({Col.DATE: 'Date',
                        Col.NARRATION1: 'Description',
                        Col.AMOUNT: 'Amount',
                        },
                       'Assets:Bank',
                       'USD',
                       '"Date","Description","Amount"',
                       )

class DummyPostingPredictor:
    DUMMY_ACCOUNT = "Assets:Dummy"

    def __init__(self, importer):
        self.importer = importer
        # move extract to _extract
        self.importer._extract = self.importer.extract
        self.importer.extract = self.extract_wrapper
    
    def extract_wrapper(self, f, existing_entries):
        entries = self.importer._extract(f, existing_entries)
        for entry in entries:
            if isinstance(entry, Transaction):
                p = entry.postings[0]
                entry.postings.append(
                    Posting(
                        account=self.DUMMY_ACCOUNT,
                        units=Amount(currency=p.units.currency, number=-1*p.units.number),
                        cost=None,
                        price=None,
                        flag=None,
                        meta={},
                    )
                )
        return entries

# wrap the importer with the dummy posting predictor.
# Will predict 2nd dummy postings for the importer entries
DummyPostingPredictor(importer)

@pytest.mark.parametrize('name', examples)
def test_source(name: str):
    check_source_example(
        example_dir=os.path.join(testdata_dir, name),
        source_spec={
            'module': 'beancount_import.source.generic_importer_source',
            'directory': testdata_csv,
            'account': 'Assets:Bank',
            'importer': importer,
        },
        replacements=[(testdata_dir, '<testdata>')])

@dumbPy dumbPy changed the title Add UNCONFORMED_ACCOUNT_KEY to imported entries Add UNCONFIRMED to externally predicted postings of imported entries Dec 23, 2020
{"source_desc":entry.narration, "date": entry.date})
postings.insert(i, p)
# add unconfirmed account key
posting.meta[UNCONFIRMED] = ""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps use True rather than "" since this seems to be a boolean field?

@jbms
Copy link
Owner

jbms commented Dec 23, 2020

Can you please add a unit test?

dumbPy and others added 2 commits January 14, 2024 13:41
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
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 dumbPy force-pushed the add-unconfirmed-account-key branch from 50c6ba9 to 278a7d0 Compare January 15, 2024 05:55
@dumbPy dumbPy force-pushed the add-unconfirmed-account-key branch from 278a7d0 to b2df616 Compare January 15, 2024 06:05
@dumbPy
Copy link
Contributor Author

dumbPy commented Jan 15, 2024

@jbms @Zburatorul Hey guys, this feels ready now.
Sorry I dropped out of the loop 4 years ago :p
Cherry-picked from #60
Now, if you wrap the importers in smart_importer's predictors,

for importer in CONFIG:
    apply_hooks(importer, [PredictPostings(), PredictPayees()])
  1. The imported transactions are added posting by PredictPostings()
  2. generic_importer_source.py adds "unconfirmed_account":True metadata to the posting.
  3. reconcile.py checks for this "unconfirmed_account":True, pops the posting, and adds FIXME posting in it's place and uses the corresponding account as dummy prediction.
  4. This dummy prediction is now passed forward as if it was predicted by the beancount-import's predictor allowing editing in the frontend.

@dumbPy
Copy link
Contributor Author

dumbPy commented Jun 8, 2024

@Zburatorul I have been using this too for a few months and works perfectly fine as expected.

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.

4 participants