PpTables
#361
Merged
PpTables
#361
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guilledk
previously approved these changes
Jul 21, 2022
In an effort to begin allowing backends to have more granular control over position updates, particular in the case where they need to be reloaded from a trades ledger, this adds a new table API which can be loaded using `open_pps()`. - offer an `.update_trans()` method which takes in a `dict` of `Transactions` and updates the current table of `Positions` from it. - add a `.dump_active()` which renders the active pp entries dict in a format ready for toml serialization and all closed positions since the last update (we might want to not drop these?) All other module-function apis currently in use should remain working as before for the moment.
In order to avoid double transaction adds/updates and too-early-discard of zero sized pps (like when trades are loaded from a backend broker but were already added to a ledger or `pps.toml` prior) we now **don't** pop such `Position` entries from the `.pps` table in order to keep each position's clears table always in place. This avoids the edge case where an entry was removed too early (due to zero size) but then duplicate trade entries that were in that entrie's clears show up from the backend and are entered into a new entry resulting in an incorrect size in a new entry..We still only push non-net-zero entries to the `pps.toml`. More fixes: - return the updated set of `Positions` from `.lifo_update()`. - return the full table set from `update_pps()`. - use `PpTable.update_from_trans()` more throughout. - always write the `pps.toml` on `open_pps()` exit. - only return table from `load_pps_from_toml()`.
Can't believe i missed this but any `return` inside a `finally` will suppress the error from the `try:` part... XD Thought i was losing my mind when the ledger was mutated and then an error just after wasn't getting raised.. lul. Never again...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reworks the
piker.ppapis to include a newopen_pps()context manager which delivers aPpTable, a type that allows real-time incremental management of position updates from backend transaction records.This more or less deprecates the previous
update_pps_conf()routine.In follow up,
both moved to #345.
- [ ] we should add support for immediate config file (ledger orpps.toml) writing likely via thetrioasync file IO apis which use a bg thread to avoid main thread latency.- [ ] probably adjust thePpTable.dump_active()output to be the open and closedPositionobject dicts and have a seperate method which generates the toml-ready "entries" dict for thepps.toml?