Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python logging switch #1305

Merged
merged 9 commits into from
Jan 10, 2024

Conversation

bug-or-feature
Copy link
Collaborator

This PR refactors away the last few usages of log.setup(). This one handles the more complex cases, where logger instances were passed around as function parameters. The solution in most cases is to refactor existing functions into class methods, where the logger instance can be accessed as a class variable instead. The changes fall into three groups:

Uses of log.setup() and specific_log() in FX and price code

  • methods _delete_fx_prices_without_any_warning_be_careful(), and _add_fx_prices_without_checking_for_existing_entry() from class parquetFxPricesData refactor away uses of log.setup() using temporary log attributes
  • broker_get_historical_futures_data_for_contract() of class ibPriceClient sets log attributes before getting price updates, and clears afterward
  • broker_get_daily_fx_data() of class ibFxClient sets log attributes before getting FX updates, and clears afterward

Uses of log_with_attributes() in the algo code

  • manage_live_trade() of class algoMarket is refactored to use log_attributes() instead of log_with_attributes()
  • top level function file_log_report_market_order() from sysexecution.algos.common_functions.py instead becomes a method of algo parent class Algo
  • top level function set_limit_price() from sysexecution.algos.common_functions.py becomes set_best_limit_price() class method of class algoOriginalBest, because parent class Algo already has a method set_limit_price() with a different method signature.
  • In class algoOriginalBest the following top level functions are converted to class methods:
    • limit_trade_viable()
    • file_log_report()
    • file_log_report_limit_order()
    • reason_to_switch_to_aggressive()
    • is_market_about_to_close()
    • required_to_switch_to_aggressive()
    • adverse_size_issue()
    • _is_imbalance_ratio_exceeded()
    • _is_insufficient_size_on_our_preferred_side()
    • set_aggressive_limit_price()
      so that they can access the parent class variable data, meaning they no longer need to be passed a logger instance
    • the new class methods adverse_size_issue(), _is_imbalance_ratio_exceeded(), and _is_insufficient_size_on_our_preferred_side() have a new parameter order so that the order attributes can be logged correctly
  • parameter broker_order_with_controls_and_order_id of method manage_live_trade() is renamed order_control for brevity
  • class methods limit_trade_viable(), reason_to_switch_to_aggressive(), is_market_about_to_close() and set_aggressive_limit_price()no longer need a data_broker parameter; they can access the parent instance.

Uses of log_with_attributes() in the order and stack handler code

  • method log_with_attributes() is removed from classes instrumentOrder, contractOrder, brokerOrder and parent Order
  • top level function calculate_adjusted_order_given_existing_orders() becomes a class method of instrumentOrderStackData so that it no longer needs to be passed a logger instance
  • method apply_broker_order_fills_to_database() of class stackHandlerForFills refactored to use temp log attributes
  • In sysexecution.stack_handler.stackHandlerCore.py, the following top level functions are converted to class methods of stackHandlerCore:
    • put_children_on_stack()
    • add_children_to_parent_or_rollback_children()
    • spawn_children_from_instrument_order()
    • log_successful_adding()
    • rollback_parents_and_children_and_handle_exceptions()
      so that they can access the parent class variable log
  • class method rollback_parents_and_children_and_handle_exceptions() is passed the parent order object instead of the parent order ID, so that the parent log attributes can be accessed (as well as the parent ID)
  • In sysexecution.stack_handler.spawn_children_from_instrument_orders.py the following top level functions are converted to class methods of class stackHandlerForSpawning:
    • spawn_children_from_instrument_order()
    • function_to_process_instrument()
    • single_instrument_child_orders()
    • adjust_limit_orders_with_correct_prices()
    • get_required_contract_trade_for_instrument()
    • child_order_in_priced_contract_only()
      so that they can access the parent class variables data and log
  • method put_children_on_stack() no longer needs to be passed parameter parent_log, the log attributes can be accessed from the parent order object, which is also passed
  • same for add_children_to_parent_or_rollback_children() and log_successful_adding()
  • the method function_to_process_instrument() would need to be refactored in future if spread orders were implemented; this has not been done for now to ease merge comparison
  • method add_instrument_and_list_of_contract_orders_to_stack() from class stackHandlerForRolls is refactored to
    use temporary log attributes, and to use the new methods put_children_on_stack(), log_successful_adding(), and rollback_parents_and_children_and_handle_exceptions()

@robcarver17
Copy link
Owner

Will look at this in January as a bit of a big one

@robcarver17 robcarver17 merged commit a268bad into robcarver17:develop Jan 10, 2024
2 checks passed
@bug-or-feature
Copy link
Collaborator Author

@robcarver17 awesome. there'll be one more PR to remove the unused functions, fix tests, update docs and tidy up

@robcarver17
Copy link
Owner

Now I have this error:

  File "/home/rob/pysystemtrade/sysdata/parquet/parquet_futures_per_contract_prices.py", line 89, in _write_prices_at_frequency_for_contract_object_no_checking
    log = futures_contract_object.log(self.log)
AttributeError: 'futuresContract' object has no attribute 'log'

I'm guessing this is a merge issue between droparctic and develop. But what is the correct way of rewriting this in new log world:

        log = futures_contract_object.log(self.log)

I haven't been following closely enough clearly, but did you drop the new_log = object.log(existing_log) pattern?

@bug-or-feature
Copy link
Collaborator Author

I just created a new PR for the final tidy up, I'll fix that issue in there

@robcarver17
Copy link
Owner

OK I've hotfixed it but not sure if it's the correct way of doing it.

@bug-or-feature
Copy link
Collaborator Author

I've pushed a fix

@robcarver17
Copy link
Owner

Thanks mate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants