feat: Add fees as crypto cost#28
Conversation
| def _sum_fees_per_year(self, transactions: List[Transaction]) -> defaultdict[int, FiatValue]: | ||
| fees_sum_per_year: defaultdict[int, FiatValue] = defaultdict(lambda: FiatValue(0)) | ||
| for transaction in transactions: | ||
| fees_sum_per_year[transaction.year()] += transaction.fees |
There was a problem hiding this comment.
| fees_sum_per_year[transaction.year()] += transaction.fees | |
| fees_in_base_currency = self.exchanger.exchange(transaction.date, transaction.fees) | |
| fees_sum_per_year[transaction.year()] += fees_in_base_currency |
_sum_fees_per_year was adding fees directly without converting them to PLN first. Some fees were in USD and others in PLN, causing the InvalidCurrencyException. So we need to exchange the fees to the base currency (PLN) before summing, just like _sum_transactions_per_year already does for transaction values
pbialon
left a comment
pbialon
left a comment
There was a problem hiding this comment.
I think we need to update the "standard" format of csv for crypto to include fees and so on.
Thanks for adding this PR, I'll create a one that takes that into account, because this one has too many merge conflicts 😅
| @click.option('--deductible-loss', '-l', default=-1, | ||
| help='Deductible loss from previous years. It overrides calculation of loss by the script', | ||
| type=float) | ||
| @click.option('--include-fees', '-fe', default=True, help='Should fees be included in the loss calculation') |
There was a problem hiding this comment.
I think fees always be included - so if they weren't it's an omission and no need to add flag/option for that.
3 participants
I've added fees as crypto cost into transaction, as according to this doc - fees should be included as cost