Unable to run interactive_update_capital_manual() tool on newly setup data

[bug-or-feature]

There's still a few problems with the recent capital changes

Traceback (most recent call last):
  File "/home/pysystemtrade/sysproduction/interactive_update_capital_manual.py", line 21, in interactive_update_capital_manual
    user_option_int = print_capital_and_get_user_input(data)
  File "/home/pysystemtrade/sysproduction/interactive_update_capital_manual.py", line 51, in print_capital_and_get_user_input
    all_calcs = data_capital.get_series_of_all_global_capital()
  File "/home/pysystemtrade/sysproduction/data/capital.py", line 45, in get_series_of_all_global_capital
    all_capital_data = self.total_capital_calculator.get_all_capital_calcs()
  File "/home/pysystemtrade/sysdata/production/capital.py", line 352, in get_all_capital_calcs
    all_capital = pd.concat(
  File "/home/anaconda3/lib/python3.8/site-packages/pandas/core/reshape/concat.py", line 271, in concat
    op = _Concatenator(
  File "/home/anaconda3/lib/python3.8/site-packages/pandas/core/reshape/concat.py", line 357, in __init__
    raise TypeError(msg)
TypeError: cannot concatenate object of type '<class 'numpy.float64'>'; only Series and DataFrame objs are valid

I think the problem is that when you initially setup capital, each value is stored as a float. But later, some other downstream code assumes a pd.Series, including manually updating the capital. In addition, there's a few places where missing or unexpected return values are not handled. For example:

• the function syscore.pdutils.uniquets() assumes that a DataFrame is passed to it, even though in some places the calling function can pass missing_data. An example of this is sysdata.production.capital.get_capital_pd_series_for_strategy()

I tried to fix this by changing uniquets() like so:

def uniquets(x):
    """
    Makes x unique
    """
    if type(x) is pd.DataFrame:
        return x.groupby(level=0).last()
    else:
        return x

But even then, in the above example, the pandas method squeeze() is called on the result, throwing another exception...

@robcarver17 when you get a moment could have a look? Maybe with your "fan of defensive programming" hat on 😃 no urgency, I have a workaround

[js190]

I fixed this locally by removing the squeeze, which is surely not what we want. I have a bunch more changes to commit as a PR.

[robcarver17]

This is a real bloody mess. The entire way total capital is stored is really complicated for no reason. I have no idea why I did it this way. It will take slightly longet, but a full rewrite of this code is required I feel.

[robcarver17]

The basic problem is that total capital is stored as 4 seperate series, rather than a single dataframe.

Everything runs through sysproduction.data.dataCapital (ah the beauties of abstraction)

    def get_percentage_returns_as_pd(self) -> pd.DataFrame:
        return self.total_capital_calculator.get_percentage_returns_as_pd()

    def get_current_total_capital(self) -> float:
        return self.total_capital_calculator.get_current_total_capital()

    def update_and_return_total_capital_with_new_broker_account_value(
        self, total_account_value_in_base_currency: float, check_limit: float = 0.1
    ) -> float:

        result = self.total_capital_calculator.update_and_return_total_capital_with_new_broker_account_value(
            total_account_value_in_base_currency, check_limit=check_limit
        )

        return result

    def get_series_of_all_global_capital(self) -> pd.DataFrame:
        all_capital_data = self.total_capital_calculator.get_all_capital_calcs()

        return all_capital_data

    def get_series_of_maximum_capital(self) -> pd.Series:
        return self.total_capital_calculator.get_maximum_account()

    def get_series_of_accumulated_capital(self) -> pd.Series:
        return self.total_capital_calculator.get_profit_and_loss_account()

    def get_series_of_broker_capital(self) -> pd.Series:
        return self.total_capital_calculator.get_broker_account()

    @property
    def total_capital_calculator(self) -> totalCapitalCalculationData:
        # cache because could be slow getting calculation method from yaml
        total_capital_calculator = getattr(self, "_total_capital_calculator", None)
        if total_capital_calculator is None:
            total_capital_calculator = self._get_total_capital_calculator()
            self._total_capital_calculator = total_capital_calculator

        return total_capital_calculator

    def _get_total_capital_calculator(self) -> totalCapitalCalculationData:
        calc_method = self.get_capital_calculation_method()
        total_capital_calculator = totalCapitalCalculationData(
            self.db_capital_data, calc_method=calc_method
        )

        return total_capital_calculator

I don't see any reason to change the idea of the total capital calculator, which both holds capital data and stores it. None of the above seems especially wrong.

So we can move down to the next level of abstraction capitalData and totalCapitalCalculationData

It's in totalCapitalCalculationData that the idea of storing seperate capital series for each type of total capital and munging them together is implemented. We could modify capitalData so it stores everything together, and then passes it out seperately, but it would be cleaner to change both the the storage object (capitalData) and the calculation object (totalCapitalCalculationData)

A key question with this kind of change to database storage is whether we make it seamless, or force the user to run a script to rewrite their capital storage.

[robcarver17]

It strikes me that it would make sense to seperate out the storage of strategy capital (which we can use the original code for) and total capital (which we want to change). Let's do that first.

[robcarver17]

On reflection, it probably makes sense to store all the capital for both strategy and total in arctic.

[robcarver17]

Pretty much done this, need to test on my production machine.

[robcarver17]

#748