Open Positions #35
Open Positions #35
Conversation
|
From a first glance, it looks pretty useful! Will do a detailed review in the weekend. |
…ecimal variable and casting AbstractTransaction variable to InTransaction
Thanks! I had some fun writing this, and it felt great to have a much faster way to get complete and accurate information that otherwise takes a long time to manually collect. The value of being able to use the data I already need to gather for taxes is a huge boon to me! |
locked column ref for ranges so rpt can be autofiltered.
There was a problem hiding this comment.
This is a cool plugin, thanks for proposing it. If you want, you may even extend it into tax loss harvesting (in a separate PR). There are a few changes that are needed but overall it looks very promising!
A few comments on the template file (since I couldn't comment inline I'll add it here):
- I think the word you need is "unrealized", not "net"
- for consistency with other plugin outputs: use full words
- for consistency with other plugin outputs: use % at the end, not pct
- "calc" is unnecessary (you already distinguish it by using yellow)
So here's the list of string changes:
- USD Net Cost Basis -> USD Unrealized Cost Basis
- Pct Portfolio CB Weigth -> Cost Basis Weight %
- USD Per Unit Calc Price -> Input Price
- USD Net Calc Value -> USD Unrealized Value
- USD Net Gain / Loss -> USD Unrealized Gain / Loss
- Pct Portfolio CB Gain / Loss -> Unrealized Gain / Loss Over Total Cost Base
- Pct Portfolio Calc Weight -> USD Unrealized Value Weight %
I'm not very sure if the "Pct Portfolio CB Gain / Loss" column is very useful: do we need it and how do you use it?
You'll need a new column for "holder": otherwise if people file together you get unintended results (e.g. all their BTC are summed up together and in the "Asset - Exchange" sheet you may see two Coinbase lines, which is confusing).
Finally, for consistency with other plugins:
- "AssetPrice" -> "Asset Price"
- "Asset_and_Exchange" -> "Asset - Exchange"
Since the "Asset" sheet is really a place where the user inputs data, it might be clearer to call it "Input".
I changed the description in the legend to be more meaningful. "Indicates the percent of which this asset’s unrealized gain/loss contributed to the whole portfolio’s Unrealized Gain / Loss %. (Gain/Loss relative to total cash investment)". I use this field along with the other 3 percentage fields (CB Weight %, Gain / Loss %, Unrealized Weight %) especially in weighing the absolute performance or lack thereof of an asset to the other assets in the portfolio. I especially compare this figure to the unrealized weight %. I use the information to help me chose where to allocate fresh funds without getting lost in the dollars and cents. I don't know if other people do this, or if it's just me, but it is nice seeing how each asset individually affects the entire portfolio's performance.
Good point. I didn't think to code this in since I've only done 1 holder per file so far. This might take additional refactoring. Per a comment on one of the items - do you want me to try to put that into this PR, or do a separate PR with that? The non-resolved items above I am requesting additional feedback on. Thanks for taking the time. -MD |
There was a problem hiding this comment.
Looking better and better! Let me know if any of your questions fell through the cracks and you're still waiting for me on something.
Adding here comments on template file:
- cell "of Total Cost Basis %" is in a different font size: best to leave it in the same font size as the rest. If the user wishes, they can adjust column size with: Edit / Select All + Format / Columns / Optimal Width.
- legend: Fill in Asset Prices in the AssetPrice Tab for calculations -> Fill in Asset Prices in the Input Tab for calculations.
- legend: Information calculated based on values entered in AssetPrice tab. -> Information calculated based on values entered in Input tab
Also add a small section for the plugin in docs/output_files.md: just a few lines describing what it does and directing the user to fill the input tab.
| input_sheet.append_rows(1) | ||
| input_row_index: int = row_indexes["Input"] | ||
| self._fill_cell(input_sheet, input_row_index, 0, asset) | ||
| self._fill_cell(input_sheet, input_row_index, 1, "See Input Tab", data_style="fiat_unit_7") |
There was a problem hiding this comment.
The "See Input Tab" text is now appearing in the Input tab, in the Asset tab and in the Asset - Exchange tab. Instead there should be: f"Enter {asset} price" in the Input tab.
There was a problem hiding this comment.
The Asset and Asset - Exchange tab have VLOOKUPs that present whatever is in the Input tab. I didn't use "Enter {asset} price" due to the risk a user would interpret that as literally entering the price into the Asset or Asset - Exchange tab directly, thus overwriting the VLOOKUP formula and causing them additional work they need not do.
Initially I figured using a default of 0, null, or "See Input Tab" would be less misleading, but I looked, and I could also set those cells to protected to help prevent the cell from being overwritten.
There was a problem hiding this comment.
I found a solution for this (let's consider line 4 as an example):
- Input tab: set B4 to
Enter Value... - Asset tab: set F4 to
=IF(IFNA(MATCH("Enter Value...", Input.B4), 0)=1, "Fill Input Sheet", Input.B4)
There was a problem hiding this comment.
Extending the formula's certainly an option too. Usually on ad hoc sheets I try to keep formulas as uncomplicated as possible, but for something generated by software, that's certainly possible.
We'd still need to use VLOOKUP since an asset with multiple holders (or multiple exchanges having balances on the Asset - Exchange tab) would break any relationship between row numbers on tabs. Essentially something like this:
=IF( VLOOKUP(A4,$Input.A:B, 2, 0)="Enter value...", "Fill Input Sheet", VLOOKUP(A4,$Input.A:B, 2, 0))
There was a problem hiding this comment.
I agree that since the output is generated we can use implementation techniques that otherwise it would be best to avoid: in a manually created spreadsheet I would also use VLOOKUP, but in generated files it's fine to use more complex formulas.
As for the Asset - Exchange tab, it's just a many to one relationship between (exchange, holder) -> asset. You can use VLOOKUP if you prefer, but I would suggest using a Python dictionary to keep track of asset -> row_number in the Input tab (so that you know which line an asset is generated at in the Input tab) and then use the dictionary to create the formulas of both the Asset and the Asset - Exchange tabs, without having to resort to VLOOKUP.
|
One more thought: I can see this plugin evolving into something more than open positions. For example, it could offer advice on tax loss harvesting or other investment management issues. Should we give it a more open-ended name, like investment_report? |
You're definitely correct about tax loss harvesting. All the information is there for someone to take advantage of already - pre-filtering that wouldn't be a big deal. It might be a lot trickier if (or when?) the US Gov decides to apply the wash sale rule to cryptos. As far as the name, I'd consider all of the reports to be investment reports: transactions, summaries, realized gains, interest, airdrops, all the rest. To me, an unrealized gains / open positions report is just another investment report to go along with the rest. It's not a big deal to me to rename it, but I do feel at least for it's current contents it is reasonably named. |
|
Ok, sounds good, let's leave the name as is. |
Converted some string literals to constants. Changed vlookup formula to show a default when input sheet not completed.
|
Hmm.. Not sure what happened on the checks there. Runs fine locally. $ git status nothing to commit, working tree clean |
|
Looks great: one more iteration and we're done! The static analysis error is probably due to a bug in the latest mypy: can you merge with the latest RP2 code? I downgraded mypy to the previous version. Also can you address the template file comments:
Finally add a small section for the plugin in docs/output_files.md: just a few lines describing what it does and directing the user to fill the input tab. |
|
Ahh yes, meant to look at that column. Good catch on those legend changes too. All changes incorporated. Feel free to tweak docs and replace images to your liking - used an example file for it. Edit: Standby before processing - just noticed a bug in a screenshot. |
|
I think these things are still missing:
|
|
Oops. Looks like I just corrected the font size on the asset/exch tab. Asset tab now updated also. Github indicates everything is up to date and doesn't seem to reflect your change to workflows. Once it does I can merge it in. |
|
Ah, looks like I had fixed it in DaLI, but not in RP2: can you update your code now and try again? |
|
Looks good to me: thanks for writing a great plugin! |
|
Awesome! My pleasure. Hope others find it as useful as I do. :) |
Here is the PR for the Open Positions #32 feature request. This does incorporate the correction regarding the use of an operation inside of the RP2Decimal instantiation, and also I've converted several magic numbers from when I was working on the code to proper defines. For now I've left some of the variable names shortened due to the wrapping that resulted from extending them. At least to these eyes, that reduced readability. Cheers!