search

AI4Finance-Foundation / FinRL-Meta

FinRL­-Meta: Dynamic datasets and market environments for FinRL.
https://ai4finance.org
MIT License
1.2k stars 560 forks source link

Create fx_training.py #218

Closed mhdmyz closed 1 year ago

zhumingpassional commented 1 year ago

Good.

I have a suggestion: this PR only adds fx_training, pls do no change other files. That is, only one file (fx_training) should be changed.

In terminal, you can execute the following command to pull the codes from master: git pull origin master

mhdmyz commented 1 year ago

I didn't change other files. Which other file you think I've changed?

zhumingpassional commented 1 year ago

In https://github.com/AI4Finance-Foundation/FinRL-Meta/pull/218/files, I can see that you changed totally 3 files.

image image image

mhdmyz commented 1 year ago

Oh no! It was just deleting and entering few lines of space to have a more tidier code! :))

zhumingpassional commented 1 year ago

It may cost reviewers to read and check if you changed them. could you pls not change other files, even deleting and entering few lines of space?

mhdmyz commented 1 year ago

Sure! But it was just for organizing and improving the readability of codes!

YangletLiu commented 1 year ago

Sure! But it was just for organizing and improving the readability of codes!

It is understandable. No worry. It is just more professional that "keep each commit to minimum change, with a self-contained functionality".

Also, you are welcome to submit a PR to organize the codes, if you see fit.

mhdmyz commented 1 year ago

Who's supposed to merge these pull requests into master?

YangletLiu commented 1 year ago

Who's supposed to merge these pull requests into master?

Currently, the team is using this rule "Each change of code should be a PR, and each PR should have someone to test it before merging."

I just added the available members (may change over time) to "Assignees", and you can interact with any one of them. When you become more familiar with the projects, I can grant you higher access to review PRs. Thanks.

It would be helpful if the PR team members can provide some feedback.