Detailed export

[Griffsano]

This is a draft for the detailed CSV export that contains all events (not only taxable ones).

@provinzio Thanks for your tips on how to approach this, they helped a lot.
As suggested, I added a boolean is_taxable to TaxEvent. Based on the new config setting EXPORT_ALL_EVENTS, either all or only taxable events are exported.
Additionally, with the new setting EXPORT_VIRTUAL_SELL, the virtual sells can also be exported to the CSV.

I also renamed some variables to avoid confusion between price and value, and added the platform to the export.

@scientes Tagging you since you were also interested in this feature.

Let me know what you think of it. Thanks! :)

Closes #94, closes #95

[scientes]

also idea: save additional price calculation information to the db (price source, which other prices were used to calculate this pricepoint etc

[provinzio]

Good idea, to export all data into a file. Herer are my 2 cents. :)

---

You differentiate your taxation types with "außerhalb des Steuerjahres" and "steuerfrei". Shouldn't this be obvious from the class variables (op.utc_time and is_taxable).

We could therefore use the "base"-taxation type and add a transaction.TaxEvent-method, which combines the base taxation type with the timestamp and whether it is taxable.

[Griffsano]

Hey @provinzio, before we tackle deposits and withdrawals (#4), I'd like to finish this PR as merging will get a mess otherwise.
For the detailed export option, the sells are now split up into multiple lines. What do you think?

[provinzio]

Hey @Griffsano,

I fixed two things (see fix commits) and removed the casefold comparison of the FIAT string as this isn't done anywhere else. I believe, that the casefold comparison might be better, but we should do this at all times and not only at the end in the evaluation phase. :) Feel free, to update all FIAT comparisons, maybe with a neat comparison function.

[provinzio]

Hey @Griffsano,

me again. I tested the code further and realized that we should tackle some more critical things before we can merge this.

---

• The variable TaxEvent.taxed_gain is currently used in multiple ways and because of that the printed evaluation at the end (print_evaluation) is wrong.

I intended to use TaxEvent.taxed_gain as the gain which is tax relevant in a specific config.TAX_YEAR (see evaluate_sell).
We can use TaxEvent.real_gain to track real gains, besides tax relevant gains (e.g. a sell of multiple coins is only partial tax relevant)

In hindsight, this makes TaxEvent.is_taxable kind of unrelevant. Do we need this at all, when we can use taxed_gain and real_gain?
I think that we should drop TaxEvent.is_taxable and stick to the two decimals taxed_gain and real_gain.

If we want to stick with TaxEvent.is_taxable we should make this a property self.taxed_gain != 0.

---

• I do not see the point in distinguishing between e.g. "Einkünfte" and "Einkünftige außerhalb des Steuerjahres". Why do we need that? All of them are (in this case) "Einkünfte", some have taxed_gains and some don't.

If we want to analysize our tax events and split them by arbitrary factors (like is in tax year) we should do this in my opinion afterwards.

Could you please explain this once more?

[provinzio]

taxed_gain and real_gain are summed up in the loop. The variables do not represent the taxed_gain oder real_gain of a single transaction.

[provinzio]

evaluate_sell shouldn't return None if we want to export all events. evaluate_sell should calculate the real_gain, which currently missing.

[Griffsano]

I'm not certain how the best way would be to implement this. To try it out, I made a new config.Fiat class for config.FIAT, with overloaded __eq__. Is this what you meant?
However, I think this would require some more changes in the code as config.FIAT is used as string everywhere, and this change might lead to unintended behavior. Just an example for Bitpanda if I overload config.Fiat:

  File "...\src\misc.py", line 185, in group_by
    d[getattr(e, key)].append(e)
TypeError: unhashable type: 'Fiat'

Similarly, if I overload core.Fiat instead and change config.py accordingly, I get an error when running Kraken dummy exports:

  File "...\src\price_data.py", line 387, in _get_price_kraken
    pair = base_asset + quote_asset
TypeError: can only concatenate str (not "Fiat") to str

Sure, we could create a custom class and overload all operators until everything is working again, but my worry is 1) this is a lot of work and 2) we can still miss something that leads to unintended behavior. Or did I do something wrong when overloading the operator? What do you think?

[Griffsano]

Regarding taxed_gain:
Just to make sure I understand real/taxed_gain correctly: If it's relevant for taxation, the value is stored in taxed_gain, else in real_gain? Or should real_gain contain taxed_gain plus any non-tax-relevant gains? In any case, I think we could drop taxed_gain and just check if taxed_gain is nonzero.

Regarding "außerhalb des Steuerjahres":
So I had two aspects in mind:

1. To make the entries in the output CSVs more transparent (if an operation took place in the respective tax year or not)
2. to prevent a mixing of e.g. "Einkünfte" and "Einkünfte außerhalb des Steuerjahres" for the sums in taxman.print_evaluation

But yes, we could also do this afterwards, would probably be cleaner. I'll think of something.

Another thing: We discussed the separation of balancing and tax evaluation (#4), do you also want to do this within this PR or a new one?

[Griffsano]

Writing it this way still confuses me^^
I know it's how the charts are named, but mathematically it should be the other way around :D
Can we write it f"Preis {price} {op.coin}-{config.FIAT}" or f"Preis ({op.coin}/{config.FIAT}) {price}" instead?

[provinzio]

mathematically... well, depending what you see.

If it is a division BTC/EUR, its 1 BTC per 1 EUR. So 2 BTC/EUR would mean, that I can buy 2 BTC for 1 EUR.

If it is "a unit mark" (dunno how this is named, often used as graph notation) BTC/EUR or writing like I painted in the picture below or I would write it as "BTC [EUR]" means, BTC in the unit of EUR. So 2 BTC/EUR means 1 BTC is the same as 2 EUR

Symbol for "Money in EUR"

Writing this "Money in EUR" with a forwardslash like Money/€ does confuse me too, but this is the way 🤷 Especially when both symbols a units by itself like km/m (which would always be 1000) or BTC/EUR. I'd perfer BTC [EUR] but no one does that, so I'd suggest we stay with the most common notation.

[provinzio]

I did just a quick test and saw, that my Sonstige Einkünfte where reduced by more than 80 % compared to a tax evaluation on master. Might be a bug. I am currently out of time, to dig deeper; just wanted to leave this here for future notice.

[provinzio]

regarding #98 (comment)

you are correct, I thought we'd already use a fiat class, but we actually only use the fiat class for selection and convert the fiat variable to string.

In that case, I'd think about extending the class Fiat (Enum). If any operation fails, it would throw an error. So I wouldn't a afraid of weird bugs coming from this.

[provinzio]

Regarding taxed_gain: Just to make sure I understand real/taxed_gain correctly: If it's relevant for taxation, the value is stored in taxed_gain, else in real_gain? Or should real_gain contain taxed_gain plus any non-tax-relevant gains? In any case, I think we could drop taxed_gain and just check if taxed_gain is nonzero.

think of it like this

  Veräußersungserlös (`selling value`)
- Anschaffungskosten (`buying cost + buying fees`)
- Werbungskosten (Transaktionskosten) (`selling fees`) 

= Gewinn/Verlust ( `real_gain`)
davon steuerbar (`taxed_gain`)

We can indeed drop the boolean is_taxable, but definitly require two floats (in this case named real_gain and taxed_gain).

Regarding "außerhalb des Steuerjahres": So I had two aspects in mind:

1. To make the entries in the output CSVs more transparent (if an operation took place in the respective tax year or not)

2. to prevent a mixing of e.g. "Einkünfte" and "Einkünfte außerhalb des Steuerjahres" for the sums in `taxman.print_evaluation`

But yes, we could also do this afterwards, would probably be cleaner. I'll think of something.

I am actually not sure, if we want to do that. This report is meant for tax authorities regarding your tax declaration in a speicifc year. Why would you want to add things to the declaration, which didn't happen in that year.

If you need this analysis for something else, we should add a config variable or write this to a separated csv file. Not the main report file.

If you want to show all transactions (for different years), run multiple tax evaluations for specific years.

Another thing: We discussed the separation of balancing and tax evaluation (#4), do you also want to do this within this PR or a new one?

I think that tax evaluation without building up a balancing queue is difficult or that otherwhise evaluating the balancing queue is the tax evaluation.

If you would want to separated the queue generation you would have to save the whole process (what is when added/removed...) so that you can calculate the taxes.

What I meant in #4 is, that we need to balance to determine which coins gets withdrawn exactly. It would be possible otherwhise.

As we need balancing for tax evaluation, too, ... I'd rather keep it together.

Doing everything twice (balancing everything twice) seems unnecessary.

[provinzio]

I do also require a more detailed export for my tax declaration, which will look somewhat different from what we export right now. We might collide a lot, working on this at the same time. I am sorry for that :x

As a heads up, I though of something like this. You might be more interest in this kind of export, too.
In general, I want an export file, which gives a better overview what is when sold and from what, with additional calculation for easy comprehension.

https://youtu.be/-8B7B4kVnmw

https://youtu.be/qV5nfFddCZU

I'll try to refactor your ideas in my approach. Have a look at the branches match-fees-with-operations and merged-detailed-export. I started by matching fees explicit to operations, as fees are only tax relevant when bound to a transaction. I think that I can start soon on the export format.

[Griffsano]

Thanks for your detailed comments. I like the examples for the detailed report.
For now, I'll not continue on this PR so we don't collide.

[provinzio]

I'll close this for now. Feel free to reopen at any time.

#4 might be a good next issue, even though the detailed export is still in progress. Matching deposit/withdrawals before the fee matching looks neccessary to me, to accomplish a complete and correct tax report (see match-fees-with-operations main.py).