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

iconfig in CSV Importer categorizer call #103

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

Conversation

Jorge1o1
Copy link

In dd50f53 the row argument was added to the CSV Importer's categorizer function. With this second argument, it became very easy to add extra Postings onto an already created Transaction.

However, this way of adding Postings is dependent on knowing the indices of columns in advance. You have to know that the "payee" column is row[2], etc. This is especially problematic if the CSV files being imported don't have columns in a deterministic order (e.g. if they're created by another Python script) or if your bank/financial institution suddenly adds or removes a column.

With this PR, the iconfig dictionary that maps column type -> index would be included as a third parameter.

def categorizer(txn, row, iconfig):
    txn = txn._replace(payee=row[iconfig[Col.PAYEE]])
    txn.meta['source'] = pformat(row)
    return txn

Caveat: workarounds exist
I know that we can use partial functions or callable class instances to bind the iconfig dictionary to our categorizer callable ahead of time. But it chafes me to have to open the file twice, normalize_config twice, etc.

@dnicolodi
Copy link
Collaborator

This CSV importer was born as an example with minimal functionality and has slowly grown into a piece of code that has more options than lines of code. It is kept in beangulp only to ease upgrade from the old importers framework. It will definitely go away after the first release of beangulp. Users should look into the new beangulp.csvbase.Importer which implements something similar with better interfaces or use petl to read and manipulate the CSV and beangulp.petl_utils to turn a petl table into a list of directives. With the intention of directing users to better alternatives, I am not very keen in adding yet another kludge to this CSV importer.

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.

2 participants