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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

@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
Collaborator

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.

None yet

5 participants