Skip to content

Commit 0c91f9a

Browse files
caballerofelipecaballerofelipe
committed
Operations reviews.
- Added tolerance to checking `stated_total` vs `calculated_total` in `buy()` and `sell()` operations. - Description is now not a parameter but fixed per operation. - Added dividend size calculation and raise exception if size is 0. - Minor changes and fixes.
1 parent 88ec454 commit 0c91f9a

File tree

2 files changed

+84
-33
lines changed

2 files changed

+84
-33
lines changed

‎TODO.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
- Create automated unit tests:
2020
- Test if operations work as expected.
2121
- Test cols_operation* methods.
22+
- Save/Load ledger from file (CSV maybe or Excel or another custom format).
23+
- Maybe the description column is redundant and the name of the operation is enough. Or maybe be more explicit in the description, instead of 'Buy {instrument}' it could be 'Buy {instrument} in exchange for {price_in}'.
24+
- Maybe bring back a column 'Q_price_commission_tax_verification' with a different name and a different purpose. The column was removed to show where there were inconsistencies between a calculated and stated total, now that's not possible since an error is raised if such an inconsistency occurs if the `tolerance_decimals=4,` is surpassed. But since we are now using a tolerance setting to allow small inconsistencies, maybe a new column would be useful to showcase that small inconsistency.
2225

2326

2427
# IDEAS

‎lib/simple_portfolio_ledger/src/simple_portfolio_ledger.py

Lines changed: 81 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ def whatis(cls, columns):
131131
elif isinstance(columns, str):
132132
return cls._ledger_columns_attrs['column notes'].get(columns, 'NOT DEFINED')
133133
else:
134-
warnings.warn('Columns must be list or str. Returning None.')
134+
warnings.warn('Columns must be list or str. Returning None.', stacklevel=2)
135135
return None
136136

137137
@classmethod
@@ -203,7 +203,7 @@ def ledger(
203203
The Ledger, with optional additional information.
204204
"""
205205
if len(self._ledger_df) == 0:
206-
warnings.warn('WARNING: Ledger is empty, showing only basic structure.')
206+
warnings.warn('WARNING: Ledger is empty, showing only basic structure.', stacklevel=2)
207207

208208
# Specifying columns to avoid returning manually added columns
209209
# `*` needed because self._ledger_columns is a tuple
@@ -706,21 +706,38 @@ def buy(
706706
tax=0,
707707
stated_total=None,
708708
date_order=None,
709-
description='',
710709
notes='',
711710
commission_notes='',
712711
tax_notes='',
712+
tolerance_decimals=4,
713713
):
714714
"""
715715
REMOVE BUT FORMAT LATER: All values are positive. Sign is changed inside the function.
716716
"""
717717
calculated_total = size * price + commission + tax
718718

719-
if stated_total is not None and stated_total != calculated_total:
720-
raise ValueError(
721-
'When adding buy operation, '
722-
+ 'the stated total is different from the calculated total.'
719+
# Allow inconsistencies up to 4 decimals
720+
if stated_total is not None and not (
721+
abs(stated_total - calculated_total) < 10**-tolerance_decimals
722+
):
723+
error_msg = ''.join(
724+
[
725+
'When adding buy operation, '
726+
+ 'the stated total is different from the calculated total. '
727+
+ f'By more than the minimum of {10**-tolerance_decimals}.'
728+
]
723729
)
730+
error_dict = {
731+
'date_execution': date_execution,
732+
'instrument': instrument,
733+
'stated_total': stated_total,
734+
'size': size,
735+
'price': price,
736+
'commission': commission,
737+
'tax': tax,
738+
'calculated_total': calculated_total,
739+
}
740+
raise ValueError(error_msg, error_dict)
724741

725742
price_w_expenses = calculated_total / size
726743

@@ -751,9 +768,9 @@ def buy(
751768
op_1['stated_total'] = -1 * calculated_total # invest
752769
op_2['stated_total'] = calculated_total # buy
753770
op_1['date_order'] = date_order if date_order is not None else date_execution # invest
754-
op_2['date_order'] = date_order if date_order is not None else date_execution # buy
755-
op_1['description'] = description # invest
756-
op_2['description'] = description # buy
771+
op_2['date_order'] = date_execution # buy
772+
op_1['description'] = f'Invest in {instrument}.' # invest
773+
op_2['description'] = f'Buy {instrument}.' # buy
757774
op_1['notes'] = notes # invest
758775
op_2['notes'] = notes # buy
759776
op_1['commission_notes'] = commission_notes # invest
@@ -774,7 +791,6 @@ def deposit(
774791
amount,
775792
account,
776793
date_order=None,
777-
description='',
778794
notes='',
779795
):
780796
"""Creates a new row with a deposit.
@@ -813,7 +829,7 @@ def deposit(
813829
'tax': 0,
814830
'stated_total': amount,
815831
'date_order': (date_order if date_order is not None else date_execution),
816-
'description': description,
832+
'description': f'Deposit {instrument}.',
817833
'notes': notes,
818834
'commission_notes': '',
819835
'tax_notes': '',
@@ -829,9 +845,25 @@ def dividend(
829845
amount,
830846
account,
831847
date_order=None,
832-
description='',
833848
notes='',
834849
):
850+
size = self._ledger_df.query(
851+
f'instrument == "{instrument_from}" and date_execution <= "{date_execution}"'
852+
)['size'].sum()
853+
854+
if size == 0:
855+
raise ValueError(
856+
' '.join(
857+
[
858+
'When doing operation "dividend",',
859+
f'the calculated size of held instrument {instrument_from} is 0.',
860+
'This could mean there are no previous operations for the instrument',
861+
'or the sum of all operations is 0. Check the ledger.',
862+
]
863+
)
864+
)
865+
866+
price = amount / size
835867
self._add_row( # dividend
836868
{
837869
'date_execution': date_execution,
@@ -840,14 +872,14 @@ def dividend(
840872
'origin': instrument_from,
841873
'destination': '',
842874
'price_in': instrument_received,
843-
'price': 1,
844-
'price_w_expenses': 1,
845-
'size': amount,
875+
'price': price,
876+
'price_w_expenses': price,
877+
'size': size,
846878
'commission': 0,
847879
'tax': 0,
848880
'stated_total': amount,
849881
'date_order': (date_order if date_order is not None else date_execution),
850-
'description': description,
882+
'description': f'Dividend from {instrument_from}.',
851883
'notes': notes,
852884
'commission_notes': '',
853885
'tax_notes': '',
@@ -867,21 +899,38 @@ def sell(
867899
tax=0,
868900
stated_total=None,
869901
date_order=None,
870-
description='',
871902
notes='',
872903
commission_notes='',
873904
tax_notes='',
905+
tolerance_decimals=4,
874906
):
875907
"""
876908
REMOVE BUT FORMAT LATER: All values are positive. Sign is changed inside the function.
877909
"""
878910
calculated_total = size * price - commission - tax
879911

880-
if stated_total is not None and stated_total != calculated_total:
881-
raise ValueError(
882-
'When adding sell operation, '
883-
+ 'the stated total is different from the calculated total.'
912+
# Allow inconsistencies up to 4 decimals
913+
if stated_total is not None and not (
914+
abs(stated_total - calculated_total) < 10**-tolerance_decimals
915+
):
916+
error_msg = ''.join(
917+
[
918+
'When adding sell operation, '
919+
+ 'the stated total is different from the calculated total. '
920+
+ f'By more than the minimum of {10**-tolerance_decimals}.'
921+
]
884922
)
923+
error_dict = {
924+
'date_execution': date_execution,
925+
'instrument': instrument,
926+
'stated_total': stated_total,
927+
'size': size,
928+
'price': price,
929+
'commission': commission,
930+
'tax': tax,
931+
'calculated_total': calculated_total,
932+
}
933+
raise ValueError(error_msg, error_dict)
885934

886935
price_w_expenses = calculated_total / size
887936

@@ -912,9 +961,9 @@ def sell(
912961
op_1['stated_total'] = -1 * calculated_total # sell
913962
op_2['stated_total'] = calculated_total # uninvest
914963
op_1['date_order'] = date_order if date_order is not None else date_execution # sell
915-
op_2['date_order'] = date_order if date_order is not None else date_execution # uninvest
916-
op_1['description'] = description # sell
917-
op_2['description'] = description # uninvest
964+
op_2['date_order'] = date_execution # uninvest
965+
op_1['description'] = f'Sell {instrument}.' # sell
966+
op_2['description'] = f'Uninvest in {instrument}.' # uninvest
918967
op_1['notes'] = notes # sell
919968
op_2['notes'] = notes # uninvest
920969
op_1['commission_notes'] = commission_notes # sell
@@ -932,10 +981,10 @@ def stock_dividend(
932981
self,
933982
date_execution,
934983
instrument,
935-
amount,
984+
price_in,
985+
size,
936986
account,
937987
date_order=None,
938-
description='',
939988
notes='',
940989
):
941990
self._add_row( # stock dividend
@@ -945,15 +994,15 @@ def stock_dividend(
945994
'instrument': instrument,
946995
'origin': instrument,
947996
'destination': '',
948-
'price_in': instrument,
997+
'price_in': price_in,
949998
'price': 0,
950999
'price_w_expenses': 0,
951-
'size': amount,
1000+
'size': size,
9521001
'commission': 0,
9531002
'tax': 0,
9541003
'stated_total': 0,
9551004
'date_order': (date_order if date_order is not None else date_execution),
956-
'description': description,
1005+
'description': f'Stock dividend from {instrument}.',
9571006
'notes': notes,
9581007
'commission_notes': '',
9591008
'tax_notes': '',
@@ -968,7 +1017,6 @@ def withdraw(
9681017
amount,
9691018
account,
9701019
date_order=None,
971-
description='',
9721020
notes='',
9731021
):
9741022
"""Creates a new row with a withdraw.
@@ -1007,7 +1055,7 @@ def withdraw(
10071055
'tax': 0,
10081056
'stated_total': -amount,
10091057
'date_order': (date_order if date_order is not None else date_execution),
1010-
'description': description,
1058+
'description': f'Withdraw {instrument}.',
10111059
'notes': notes,
10121060
'commission_notes': '',
10131061
'tax_notes': '',
@@ -1070,7 +1118,7 @@ def set_instruments_metadata(self, instuments_metadata: dict) -> None:
10701118
Parameters
10711119
----------
10721120
instuments_metadata : dict
1073-
The required format is the following:👍🏼
1121+
The required format is the following:
10741122
```json
10751123
{instrument_name:
10761124
{'type': instrument_type,

0 commit comments

Comments
 (0)