Skip to content

remove all the print() statements and use some form of logging instead#196

Closed
chennnuo wants to merge 2 commits intoAI4Finance-Foundation:masterfrom
chennnuo:staging
Closed

remove all the print() statements and use some form of logging instead#196
chennnuo wants to merge 2 commits intoAI4Finance-Foundation:masterfrom
chennnuo:staging

Conversation

@chennnuo
Copy link

The following functions have been modified:

  • plot.py
  • test.py
  • train.py
  • tutorials/1-Introduction/China_A_share_market_tushare.py
  • tutorials/2-Advance/Crypto_Feature_Importance.py
  • tutorials/2-Advance/FinRL_PortfolioAllocation_Explainable_DRL.py
  • tutorials/2-Advance/execution_optimizing.py
  • tutorials/2-Advance/nas100_gpu_podracer.py
  • tutorials/2-Advance/stock_jq.py

@zhumingpassional
Copy link
Contributor

the following can be written a function?
logger = logging.getLogger()
logger.setLevel("DEBUG")
file_handler = logging.FileHandler("./log.txt", mode="a", encoding="utf-8")
file_handler.setLevel("DEBUG")
file_handler.setFormatter(
logging.Formatter(fmt="%(lineno)s---%(asctime)s---%(message)s")
)
logger.addHandler(file_handler)

@zhumingpassional
Copy link
Contributor

have you revised all print() to logging?

@chennnuo
Copy link
Author

have you revised all print() to logging?

I have checked all run files at present, and I will continue to check other files in the package later.

@chennnuo
Copy link
Author

Thanks for your advice. I'm doing this, I have a question, which folder would be more appropriate for the "log" function to be placed? meta? Or create a new folder, like utils?

@zhumingpassional
Copy link
Contributor

i prefer creating a log folder.

@hawkeye-bot
Copy link
Contributor

hawkeye-bot commented Jul 13, 2022

Good step to replace all the print functions by logging-statements!

May I suggest 2 changes to the logging stuff?

  1. place the log initialization in a generic function somewhere as @zhumingpassional suggests
  2. load log configuration from a logging file (example code below). This allows for easy modification to the log setup, without having to modify the code itself. Just a suggestion :)

with open(args.logging_config_file, 'rt') as file: logging_config = yaml.safe_load(file.read()) try: logging_config['handlers']['file_handler']['filename'] = args.logfile except Exception: pass # just ignore it logging.config.dictConfig(logging_config)

@zhumingpassional
Copy link
Contributor

@hoeckxer good suggestions. thanks. glad to work with you.

@chennnuo pls revise it as the above suggestions. thanks.

@eyast
Copy link
Contributor

eyast commented Jul 14, 2022

Hey guys how about using a log Decorator?

https://ankitbko.github.io/blog/2021/04/logging-in-python/

@chennnuo
Copy link
Author

Thank you very much for everyone's suggestions, I will take your suggestions seriously to revise and improve!

@chennnuo chennnuo closed this Jul 14, 2022
@chennnuo chennnuo deleted the staging branch July 14, 2022 07:34
@zhumingpassional
Copy link
Contributor

Hey guys how about using a log Decorator?

https://ankitbko.github.io/blog/2021/04/logging-in-python/

@eyast impressive idea.

@chennnuo
you do not need to close it.
you can revise PR locally and push it again.

@chennnuo chennnuo restored the staging branch July 14, 2022 07:51
@chennnuo chennnuo reopened this Jul 14, 2022
@chennnuo
Copy link
Author

Oh sorry, I'll reopen it right away, revise PR locally and push it again as soon as I'm done modifying and testing.

@YangletLiu
Copy link
Contributor

@chennnuo Just to check that this is fixed in the PR #195
right? Do we need a further operation?

@chennnuo
Copy link
Author

@XiaoYangLiu-FinRL Sorry, I haven't finished fixing it yet, I'll fix it soon and let you know.

@YangletLiu
Copy link
Contributor

@chennnuo No problem. Remember to use the most updated branch, to avoid conflict. Thanks.

@chennnuo
Copy link
Author

@XiaoYangLiu-FinRL Ok thanks for the heads up, I'll keep an eye on that.

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.

5 participants