Fix #1560: Reduce and improve db queries #1562
Conversation
| predictoor_cols, predictoor_data, raw_predictoor_data = ( | ||
| self.get_data_for_predictoors_table(predictoor_stats) | ||
| self.get_data_for_predictoors_table(self.app.predictoors_data) |
There was a problem hiding this comment.
you can use the same self.app.predictoors data in self.get_data_for_predictoors_table, since it's the same instance. No need to pass it around as a parameter
| @@ -54,7 +54,7 @@ def sample_table_rows(): | |||
|
|
|||
| def pytest_setup_options(): | |||
| options = Options() | |||
| options.add_argument("--headless") | |||
| # options.add_argument("--headless") | |||
There was a problem hiding this comment.
please reinstate this, we do not need to run visual stuff on the CI machines
| @@ -71,7 +75,8 @@ def get_predictoors_data_from_payouts( | |||
| list: List of processed user payouts stats data. | |||
| """ | |||
|
|
|||
| for data in user_payout_stats: | |||
| payout_stats = deepcopy(user_payout_stats) | |||
| for data in payout_stats: | |||
| formatted_data = format_dict( | |||
There was a problem hiding this comment.
I would suggest format_dict had an extra argument "only_include_keys" which does pick from dict implicitly, and since pick from dict is really simple, we don't need an explicit function, just the functionality in format_dict.
I also think this goes well, logically speaking, into the responsibility of formatting the dict
| } | ||
|
|
||
| return { | ||
| "avg_accuracy": 0, |
There was a problem hiding this comment.
I'm thinking maybe we should have some defaults in the result itself? Just an idea, not part of the main code review
| ) -> Dict[str, Union[float, int, str]]: | ||
| result = find_with_key_value(feed_subcription_stats, "contract", contract) | ||
|
|
||
| if result: |
There was a problem hiding this comment.
can you treat the other case first and unindent?
| f"{result['ws_buy_count']}-WS" if result["ws_buy_count"] > 0 else "" | ||
| ) | ||
|
|
||
| if df_buy_count_str or ws_buy_count_str: |
There was a problem hiding this comment.
or alternately build the result itself. It could also improve this part right here modify result["sales"] if needed
| } | ||
|
|
||
|
|
||
| def get_feeds_data_for_feeds_table( |
There was a problem hiding this comment.
this is an excellent candidate of why presentation should go into db getter: it's just renaming some columns and presenting them differently. (Not part of the main code review, but discussed for a further iteration)
| @@ -43,7 +43,6 @@ | |||
| "time_machine==2.15.0", | |||
| "typeguard==4.3.0", | |||
| "xgboost==2.1.1", | |||
| "dash_bootstrap_components==1.6.0", | |||
There was a problem hiding this comment.
are we sure we don't need this? Aren't we using it in other dashboards?
There was a problem hiding this comment.
it's a duplicate, that's why a removed it
Fixes #1560