pfalcon / ppxml2db

Scripts to import PortfolioPerformance (https://github.com/portfolio-performance/portfolio) XML into a SQLite DB and export back
10 stars 0 forks source link

crossEntry Failure #8

Closed flywire closed 2 months ago

flywire commented 3 months ago

The NotImplementedError is clear. Is any information available to understand the crossEntry failure? Any debug or hack suggestions to try and work through it?

Could this just be transfers between Cash accounts or Security accounts? If so transactions could be changed.

Commenting out the crossEntry section of the code (ppxml2db.py:282-306) successfully creates the DB with all other tables. What are the implications of converting that DB to an xml, even if possible?

ppxml2db2.py shown is a mod with the extra dot added to ppxml2db.py:283.

E:\sqlite>python ppxml2db.py private.xml private.db
2024-07-01 11:20:45 INFO  Handling <security>
2024-07-01 11:20:48 INFO  Handling <watchlist>
2024-07-01 11:20:48 INFO  Handling <account>
2024-07-01 11:20:48 INFO  Handling <portfolio>
2024-07-01 11:20:48 INFO  Handling <account-transaction>
2024-07-01 11:20:48 INFO  Handling <portfolio-transaction>
2024-07-01 11:20:48 INFO  Handling <crossEntry>
E:\sqlite\ppxml2db.py:283: FutureWarning: This search incorrectly ignores the root element, and will be fixed in a future version.  If you rely on the current behaviour, change it to './/crossEntry'
  for x_el in self.etree.findall("//crossEntry"):
Traceback (most recent call last):
  File "E:\sqlite\ppxml2db.py", line 397, in <module>
    conv = PortfolioPerformanceXML2DB(root)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\sqlite\ppxml2db.py", line 296, in __init__
    raise NotImplementedError
NotImplementedError

E:\sqlite>python ppxml2db2.py private.xml private.db
2024-07-01 12:28:18 INFO  Handling <security>
2024-07-01 12:28:21 INFO  Handling <watchlist>
2024-07-01 12:28:21 INFO  Handling <account>
2024-07-01 12:28:21 INFO  Handling <portfolio>
2024-07-01 12:28:21 INFO  Handling <account-transaction>
2024-07-01 12:28:21 INFO  Handling <portfolio-transaction>
2024-07-01 12:28:21 INFO  Handling <crossEntry>
Traceback (most recent call last):
  File "E:\sqlite\ppxml2db2.py", line 397, in <module>
    conv = PortfolioPerformanceXML2DB(root)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\sqlite\ppxml2db2.py", line 296, in __init__
    raise NotImplementedError
NotImplementedError

E:\sqlite>python --version
Python 3.12.4

E:\sqlite>python ppxml2db2.py --version
ppxml2db2.py 0.5

E:\sqlite>ver

Microsoft Windows [Version 10.0.22631.3737]

E:\sqlite>sqlite3
SQLite version 3.36.0 2021-06-18 18:36:39

E:\sqlite>find /n "version" private.xml

---------- PRIVATE.XML
[2]  <version>62</version>

Portfolio Performance Version: 0.69.0 (June 2024)


test.bat

@echo on
if %1!==! goto usage
if exist %1.db del %1.db
if exist tmp.txt del tmp.txt
rem for /f "tokens=*" %%f in (tables.txt) do echo .read %%f.sql >>tmp.txt
for %%f in (*.sql) do echo .read %%f >>tmp.txt
sqlite3 -init tmp.txt %1.db .quit
rem del tmp.txt
python ppxml2db.py %1.xml %1.db
python db2ppxml.py %1.db %12.xml
goto end
:usage
@echo off
echo.
echo Usage: %0 filename(without extension)
echo.
:end
flywire commented 3 months ago

Got it: account-transfer and portfolio-transfer

Why does account-transfer raise NotImplementedError?


        _log.info("Handling <crossEntry> - Skipped")
        for x_el in self.etree.findall("//crossEntry"):
            if x_el.get("reference") is not None:
                continue
            typ = x_el.get("class")
            if typ == "buysell":
                fields = {
                    "type": typ,
                    "portfolio": self.uuid(x_el.find("portfolio")),
                    "portfolio_xact": self.uuid(x_el.find("portfolioTransaction")),
                    "account": self.uuid(x_el.find("account")),
                    "account_xact": self.uuid(x_el.find("accountTransaction")),
                }
                dbhelper.insert("xact_cross_entry", fields, or_replace=True)
#           elif typ == "account-transfer":
#               raise NotImplementedError
#               fields = {
#                   "type": typ,
#                   "accountFrom": self.uuid(x_el.find("accountFrom")),
#                   "accountFrom_xact": self.uuid(x_el.find("transactionFrom")),
#                   "account": self.uuid(x_el.find("accountTo")),
#                   "account_xact": self.uuid(x_el.find("transactionTo")),
#               }
            else:
                print(typ)
#               raise NotImplementedError
#           dbhelper.insert("xact_cross_entry", fields, or_replace=True)
E:\sqlite>python ppxml2db.py private.xml private.db
2024-07-01 19:34:18 INFO  Handling <security>
2024-07-01 19:34:21 INFO  Handling <watchlist>
2024-07-01 19:34:21 INFO  Handling <account>
2024-07-01 19:34:21 INFO  Handling <portfolio>
2024-07-01 19:34:21 INFO  Handling <account-transaction>
2024-07-01 19:34:21 INFO  Handling <portfolio-transaction>
2024-07-01 19:34:21 INFO  Handling <crossEntry> - Skipped
E:\sqlite\ppxml2db.py:283: FutureWarning: This search incorrectly ignores the root element, and will be fixed in a future version.  If you rely on the current behaviour, change it to './/crossEntry'
  for x_el in self.etree.findall("//crossEntry"):
account-transfer
...
portfolio-transfer
...
2024-07-01 19:34:21 INFO  Handling <taxonomy>
2024-07-01 19:34:21 INFO  Handling <dashboard>
2024-07-01 19:34:21 INFO  Handling <properties>
2024-07-01 19:34:21 INFO  Handling <settings>
pfalcon commented 3 months ago

Why does account-transfer raise NotImplementedError?

Perhaps because it was not implemented? I don't have a need for such transactions, and briefly looking into them, I couldn't get a proper round-trip. You're welcome to comment out that raise and see how it works for you, and report results. And if anything doesn't work, there would be a need for very minimal XML showing just one account-transfer (perhaps with a couple of other xacts), to serve as a reference for development/testing. But I'm not sure if I'd have time to look into that anytime soon.

flywire commented 3 months ago

Why does account-transfer raise NotImplementedError?

I couldn't get a proper round-trip. You're welcome to comment out that raise and see how it works for you, and report results.

test-account-transfer.zip

The original PP xml file has transaction details as a TRANSFER_IN and the db2ppxml.py file has transaction details as a TRANSFER_OUT.


code mod used:

        _log.info("Handling <crossEntry> - Skipped")
        for x_el in self.etree.findall("//crossEntry"):
            if x_el.get("reference") is not None:
                continue
            typ = x_el.get("class")
            if typ == "buysell":
                fields = {
                    "type": typ,
                    "portfolio": self.uuid(x_el.find("portfolio")),
                    "portfolio_xact": self.uuid(x_el.find("portfolioTransaction")),
                    "account": self.uuid(x_el.find("account")),
                    "account_xact": self.uuid(x_el.find("accountTransaction")),
                }
                dbhelper.insert("xact_cross_entry", fields, or_replace=True)
            elif typ == "account-transfer":
#               raise NotImplementedError
                fields = {
                    "type": typ,
                    "accountFrom": self.uuid(x_el.find("accountFrom")),
                    "accountFrom_xact": self.uuid(x_el.find("transactionFrom")),
                    "account": self.uuid(x_el.find("accountTo")),
                    "account_xact": self.uuid(x_el.find("transactionTo")),
                }
                dbhelper.insert("xact_cross_entry", fields, or_replace=True)
            else:
                print(typ)
#               raise NotImplementedError
#           dbhelper.insert("xact_cross_entry", fields, or_replace=True)
flywire commented 3 months ago

Looking at the file in PP it transferred out to the same file but it is screwy:

image

test.db xact table shows transfer out and in to the same account too.

flywire commented 3 months ago

This round-trip xlm compare clearly shows account-transfer account-transaction was inserted before the crossEntry rather than after. Fixing the db xact table doesn't fix it:

test-compare

flywire commented 3 months ago

To be explicit, the <crossEntry> object in an <account-transaction> (if any) must occur after <amount> and before <shares>. It was not inserted correctly for the second <account-transaction> shown above.

flywire commented 3 months ago

Drag and drop shows data just needs writing in the correct order fixing some of the tags and references.

image

pfalcon commented 3 months ago

Ok, I pushed v.0.6 which handles XML from test-account-transfer.zip attached above properly. Please test it on more realistic account transfer examples.

And there certainly will be similar (if not worse) situation with portfolio transfers, again, feel free to provide a simple repro testcase.

flywire commented 3 months ago

Further testing in KommerA round trips successfully (except line endings in Windows).

Still, it is flakey (tested without portfolio transfers) so more test data required:

E:\sqlite>python db2ppxml.py --version private.db private2.xml
db2ppxml.py 0.6

E:\sqlite>python db2ppxml.py --debug private.db private2.xml
...
DEBUG:dbhelper:SELECT * FROM xact_cross_entry WHERE account_xact='ee5ca18e-0b38-474c-a184-b44c693e3976' OR accountTo_xact='ee5ca18e-0b38-474c-a184-b44c693e3976'
Traceback (most recent call last):
  File "E:\sqlite\db2ppxml.py", line 443, in <module>
    main()
  File "E:\sqlite\db2ppxml.py", line 341, in main
    make_account(etree, accounts, acc_r)
  File "E:\sqlite\db2ppxml.py", line 192, in make_account
    make_xacts(etree, xacts, acc_r["uuid"])
  File "E:\sqlite\db2ppxml.py", line 157, in make_xacts
    make_xact(etree, pel, tag, xact_r)
  File "E:\sqlite\db2ppxml.py", line 119, in make_xact
    assert try_ref(etree, rf, x_r["account"])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

<forex and <exchangeRate> are missing in KommerB.xml:

image

kommer+A+B.zip

flywire commented 3 months ago

This fails and is the flakey I refer to above. It is a reproduction of something in my private file, two transfers (outbound) and a transfer (inbound), with the last transfer generating the error:

E:\sqlite>python ppxml2db.py kommera1.xml kommera1.db
2024-07-04 20:45:07 INFO  Handling <security>
2024-07-04 20:45:07 INFO  Handling <watchlist>
2024-07-04 20:45:07 INFO  Handling <account>
2024-07-04 20:45:07 INFO  Handling <portfolio>
2024-07-04 20:45:07 INFO  Handling <account-transaction>
2024-07-04 20:45:07 INFO  Handling <portfolio-transaction>
2024-07-04 20:45:07 INFO  Handling <crossEntry>
E:\sqlite\ppxml2db.py:283: FutureWarning: This search incorrectly ignores the root element, and will be fixed in a future version.  If you rely on the current behaviour, change it to './/crossEntry'
  for x_el in self.etree.findall("//crossEntry"):
2024-07-04 20:45:07 INFO  Handling <taxonomy>
2024-07-04 20:45:08 INFO  Handling <dashboard>
2024-07-04 20:45:08 INFO  Handling <properties>
2024-07-04 20:45:08 INFO  Handling <settings>

E:\sqlite>python db2ppxml.py kommera1.db kommera12.xml
{'type': 'account-transfer', 'portfolio': None, 'portfolio_xact': None, 'account': 'e068fb14-2554-427e-b2d0-30dcc6e15717', 'account_xact': '40661deb-c66d-49f0-b362-ac414730d920', 'accountTo': 'ea9414e0-1787-46c0-92b3-8e2370eb892e', 'accountTo_xact': '9b43aafd-f4cc-4e88-ae4d-85c6c1578a34'}
{'type': 'account-transfer', 'portfolio': None, 'portfolio_xact': None, 'account': 'ea9414e0-1787-46c0-92b3-8e2370eb892e', 'account_xact': '9a3745dd-f00b-4265-98c0-5eb9dd53992f', 'accountTo': 'e068fb14-2554-427e-b2d0-30dcc6e15717', 'accountTo_xact': 'ed0f19d6-a1db-4eb2-8b3b-059df291c28c'}
{'type': 'account-transfer', 'portfolio': None, 'portfolio_xact': None, 'account': 'ea9414e0-1787-46c0-92b3-8e2370eb892e', 'account_xact': '20e131cb-51cd-4354-ae95-ba7fec866eda', 'accountTo': 'e068fb14-2554-427e-b2d0-30dcc6e15717', 'accountTo_xact': '648b70b2-0db6-49c7-89c5-488f0db8992f'}
{'type': 'account-transfer', 'portfolio': None, 'portfolio_xact': None, 'account': 'ea9414e0-1787-46c0-92b3-8e2370eb892e', 'account_xact': 'efdebe25-d861-4369-b59e-8049df48ef1f', 'accountTo': 'e068fb14-2554-427e-b2d0-30dcc6e15717', 'accountTo_xact': '4252eae7-3e94-4609-93ed-118f4f8bc6b6'}
{'type': 'account-transfer', 'portfolio': None, 'portfolio_xact': None, 'account': 'e068fb14-2554-427e-b2d0-30dcc6e15717', 'account_xact': 'f1ffa95f-2760-4a13-ba64-e10c75388a7e', 'accountTo': 'ea9414e0-1787-46c0-92b3-8e2370eb892e', 'accountTo_xact': '98133384-ef8e-4975-83fe-91ca28cf6b7c'}
Traceback (most recent call last):
  File "E:\sqlite\db2ppxml.py", line 443, in <module>
    main()
  File "E:\sqlite\db2ppxml.py", line 341, in main
    make_account(etree, accounts, acc_r)
  File "E:\sqlite\db2ppxml.py", line 192, in make_account
    make_xacts(etree, xacts, acc_r["uuid"])
  File "E:\sqlite\db2ppxml.py", line 157, in make_xacts
    make_xact(etree, pel, tag, xact_r)
  File "E:\sqlite\db2ppxml.py", line 123, in make_xact
    make_account(etree, x, accto_r, el_name="accountTo")
  File "E:\sqlite\db2ppxml.py", line 192, in make_account
    make_xacts(etree, xacts, acc_r["uuid"])
  File "E:\sqlite\db2ppxml.py", line 157, in make_xacts
    make_xact(etree, pel, tag, xact_r)
  File "E:\sqlite\db2ppxml.py", line 121, in make_xact
    assert try_ref(etree, rf, x_r["account_xact"])
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

kommerA1.zip

Note: db2ppxml.py in zip file has an extra print line.

E:\sqlite>python ppxml2db.py --version ppxml2db.py 0.6

Summary:

image

pfalcon commented 3 months ago

I'd like to see an xml file generated with something

Go for it! You said you have some Python programming experience, and actually you would need some programming experience to use this tool, because in the end, you get a database, and you need to write a script to do something useful with it. In this case, you remove "assert" (just the keyword, not its argument, which is again should be obvious if you have some programming experience) - and see how far that will let you go.

Linux line endings would be useful for quick filesize check

I don't do anything special about line ending. I might, but then just imagine a next user comes and says: "Hey, you generate Unix endings on Windows!". So, I'd like to be 100% sure that PP writes XMLs with Unix line-endings (I find that strange). Feel free to prove that to me ;-).

does not run if --version option used

Not supposed to.

pfalcon commented 3 months ago

I pushed 0.6.1 which round-trips kommerA1.xml. Thanks for preparing testcases.

flywire commented 3 months ago

Working through those comments in reverse.

  1. New version db2ppxml.py is failing at private.xml first account transfer now (maybe I'd removed that previously for testing). I'll look at it further.
  2. tbh, both our time is better spent with me working on tests, since my programming days I've done a lot of testing
  3. I don't really care about --version but [understandably] you want environment information (OS/python/tool version) with any failures, easy if tool generates it at runtime
  4. Round trip files including final PP price update run on windows - note linux line endings kommerA12.PPUpdated.zip
  5. I've only seen assert used in tests and I can't see the benefit of just ending execution with the failing line number. Thanks for the minimal guidance on how to work through it.

    Minimal portfolio-transfer test file: test20.zip which just ends at the assert.

pfalcon commented 3 months ago

In the meantime, I implemented partial support for FX info in xact units (kommerB.xml).

flywire commented 3 months ago

Security transaction Buy/Sell:

  1. notes have the &#xd; replaced with a blank line
  2. <source> added by import pdf bank documents is dropped from round trip
  3. fairly sure <configurationSets> reporting-periods are dropped too
flywire commented 3 months ago

This seems to demonstrate the flakey part of regenerating the xml (ppxml2db.py 0.6.2). DemoA.zip is based on Import CSV file.

xml file is not regenerated correctly with all Cash Account Bank transactions generated in Cash Account Broker Funds. ~Issue seems to have something to do with associated security transactions.~

image

image

flywire commented 2 months ago

The flakiness can be demonstrated much simpler using testA.zip modified from an earlier comment by making the Deposit and Transfer (Outbound) with Cash2 instead of Cash Deposit Accounts:

image image

The DemoA.xml in the above comment would still be a useful test.

pfalcon commented 2 months ago

The flakiness

I perplexed by your comments. "Flaky" means erratic, non-deterministic behavior, i.e. on one run it produces one output, on another - different. Is that the behavior that you see? Where are textual diffs for different cases?

pfalcon commented 2 months ago

This ticket has too many different issues reported, so it's hard to follow thru it. I'll try to collect all the testcases attached here and look thru them, but I'd suggest to open new tickets for new issues (and single ticket per issue).

pfalcon commented 2 months ago

In the meantime, I made changes to fully round-trip kommerB.xml (which is an example of issue not related to the original topic of this ticket, "crossEntry Failure").

pfalcon commented 2 months ago

Ok, Unix line-endings in the latest 0.7.1.

But yeah, it takes more effort to wade thru comments here than to make fixes like that. So, let me close this ticket, and please open new, separate, per-issue tickets for each new stuff. (As I told, I'll try to collect all the testcase posted here already.)