ta-oliver / infertrade

Open source trading and investment strategy library designed for accessibility and compatibility
Apache License 2.0
34 stars 20 forks source link

all allocation function in ta_export_regression_allocation are displaying same behaviour #216

Closed bi-kash closed 3 years ago

bi-kash commented 3 years ago

Problematic code:

allocation_dictionary = {}

for ii_ta_signal in ta_export_signals:
    ta_rule_name = ta_export_signals[ii_ta_signal]["function_names"]

    def allocation_function(time_series_df: pd.DataFrame, *args, **kwargs) -> pd.DataFrame:
        """Generic regression treatment of ta technical signals."""
        ta_signal_func = ta_export_signals[ii_ta_signal]["class"]

        adapted_allocation_rule_using_regression = ta_adaptor(ta_signal_func, ta_rule_name, *args, **kwargs)

        pipeline = make_pipeline(
            FunctionTransformer(adapted_allocation_rule_using_regression),
            PricePredictionFromSignalRegression(),
            PositionsFromPricePrediction(),
        )

        time_series_df = pipeline.fit_transform(time_series_df)
        return time_series_df

    ta_regression_name = ta_rule_name + "_regression"

    dictionary_addition = {
        ta_regression_name: {
            "function": allocation_function,
            "parameters": ta_export_signals[ii_ta_signal]["parameters"],
            "series": ta_export_signals[ii_ta_signal]["series"],
            "available_representation_types": {
                "github_permalink": "https://github.com/ta-oliver/infertrade/blob/f571d052d9261b7dedfcd23b72d925e75837ee9c/infertrade/algos/community/allocations.py#L282"
            },
        }
    }

    allocation_dictionary.update(dictionary_addition)

return allocation_dictionary

All the allocation_function in allocation_dictionary are showing the same behavior i.e. calculating allocations using the same indicator.

It's so because ta_signal_func and ta_rule_name are not local to function. One of the solutions can be adding ta_signal_func and ta_rule_name as a default argument. But we want to avoid that to maintain consistency with other allocation functions.

@SaurabhBansalInferstat This is the reason why setting parameters is giving an error on inferlib side.

@ta-oliver Any suggestions?

ta-oliver commented 3 years ago

Can we deepcopy, so that each version is separately defined, as per https://github.com/ta-oliver/infertrade/pull/217/files?

bi-kash commented 3 years ago

Can we deepcopy, so that each version is separately defined, as per https://github.com/ta-oliver/infertrade/pull/217/files?

Please take a look at the review at the same PR