isaacharrisholt / quiffen

Quiffen is a Python package for parsing QIF (Quicken Interchange Format) files.
MIT License
34 stars 30 forks source link

Write some fewer empty records #46

Closed teeberg closed 1 year ago

teeberg commented 1 year ago

I'm trying to generate QIF files to import into GnuCash and found two issues that made this process a little bumpy. This is one of them.

GnuCash was crashing on some of the QIF files that I created but didn't spit out any useful (to me, anyway) error messages.

Looking at the generated QIF in more detail, I noticed that there were instances of consecutive lines each consisting of just the ^, indicating the end/beginning of a record, i.e. there were empty records.

One could say GnuCash should simply skip over those, but I think there's no reason why quiffen should write empty records. So if there's a chance for us to write records here that will be accepted by more software such as GnuCash, I think we should do that! 😊

It turns out that if there are no accounts, categories or classes, then code like this:

qif += '^\n'.join(
            cls.to_qif()
            for cls in self.classes.values()
        ) + '^\n'

will always result in at least the trailing ^\n, without actually having written any records. This results in an empty record if there were e.g. no classes to write.

I hope you will accept this PR! :) Let me know if there's anything else you'd like me to do!

teeberg commented 1 year ago

@isaacharrisholt I added another test commit before the "fix" commit to let you test the change more easily:

If you git checkout c43b7adbb05447f034802d7ca91bf4bd7a211012, and run pytest tests/test_qif.py::test_empty_qif, the test fails:

___________________________________________________________ test_empty_qif ____________________________________________________________

    def test_empty_qif():
        qif = Qif()
>       assert qif.to_qif() == ''
E       AssertionError: assert '^\n^\n^\n' == ''
E         + ^
E         + ^
E         + ^

tests/test_qif.py:1087: AssertionError

On this branch, after the fix, that test passes.

Would you like to see something else?

I hope you don't mind my re-ordering/rebasing :)

isaacharrisholt commented 1 year ago

This all looks good! Thank you for your contribution!