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

Ensure the CSV file is always closed correctly #5

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

omarkohl
Copy link
Contributor

Since currently this is not possible when using a file as parameter (see
kedder/ofxstatement#71) the easiest workaround
is to simply pass the file content to the parser. The csv.reader() takes
any iterable (such as a list of lines, as in this case) as input.

Since currently this is not possible when using a file as parameter (see
kedder/ofxstatement#71) the easiest workaround
is to simply pass the file content to the parser. The csv.reader() takes
any iterable (such as a list of lines, as in this case) as input.
parser = FrankfurterSparkasse1822Parser(f)
with open(filename, 'r', encoding=encoding) as f:
lines = f.readlines()
parser = FrankfurterSparkasse1822Parser(lines)
parser.statement.account_id = self.settings['account']
parser.statement.bank_id = self.settings.get('bank', '50050201')
parser.statement.currency = self.settings.get('currency', 'EUR')
Copy link
Owner

Choose a reason for hiding this comment

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

You are right, this is as bug. But honestly, I do not like the fix, because it forces all data into the memory.

I wonder if this would be better ....

class AutoClose:
    def __init__(self, f):
        self.__f = f
    def __del__(self):
        self.__f.close()
    def __getattr__(self, name):
        return getattr(self.__f, name)

...

    f = AutoClose(open(....))

Do you know how other parsers are handling this, I did not check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this is a downside but nowadays I usually assume that memory is not a problem (with GBs and GBs of it) unless proven otherwise. Premature optimization and so on ;-) Who has bank statements that are several hundred MB big? This program will be executed once and then exit. It won't run as a daemon in the background...

I think __del__ is not guaranteed to be called or may even be called multiple times.

I think my solution is quite straightforward and explicit, which is also an advantage.

Hopefully the issue I opened in the ofxstatement project will get adressed.

I didn't check how other plugins are solving this issue.

Copy link
Owner

Choose a reason for hiding this comment

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

Well, You are right. __del__ is not guaranteed to be called. I think I need to write test for this plugin.

@MirkoDziadzka MirkoDziadzka merged commit facc14c into MirkoDziadzka:master Mar 14, 2018
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