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

Add tests #190

Closed eyast closed 2 years ago

eyast commented 2 years ago

I have added several tests:

eyast commented 2 years ago

Hi @zhumingpassional - let me know if you have any questions before merging this PR, thanks

eyast commented 2 years ago

Hi @Athe-kunal - this is the PR that I've mentioned. Please check out the result of the first test case (the one above pre-commit.ci). Once it completes, it should fail, with some failures related to get_data() and get_trading_days(). can you help create issues in the project tracker so we start resolving those? Thanks!!

zhumingpassional commented 2 years ago

@eyast

  1. in test_yahoo.py, i am wondering the reason of building a class not a function just like test_baostock, which includes access data, clean data, and add technical indicators?
  2. test_joinquant may need to be added.
  3. some test cases, e.g., joinquant, test_tushare, need account/passwd, therefore, kwargs should be one parameter. even kwargs is added as a param, the test code is open, how to make sure that others do not know the account/passwd is also a problem.
eyast commented 2 years ago

The real difference is that the Yahoo test cases are built on top of pytest - a testing framework. Test_baostock is not built on top of a testing framework. We need a testing framework. Class or not class is not very relevant - both work


From: Ming Zhu @.> Sent: Sunday, July 10, 2022 12:46:09 PM To: AI4Finance-Foundation/FinRL-Meta @.> Cc: Eyas Taifour @.>; Mention @.> Subject: Re: [AI4Finance-Foundation/FinRL-Meta] Add tests (PR #190)

@eyasthttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Feyast&data=05%7C01%7Ceyast%40microsoft.com%7C27fd3026e95a498cd21008da6259053c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637930431751806009%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2FXnVaclTbsYWkI4OJjCoRKhWFo31bHpdCjkiduUz%2B2U%3D&reserved=0 in test_yahoo.py, i am wondering the reason of building a class not a function just like test_baostock, which includes access data, clean data, and add technical indicators?

— Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAI4Finance-Foundation%2FFinRL-Meta%2Fpull%2F190%23issuecomment-1179694287&data=05%7C01%7Ceyast%40microsoft.com%7C27fd3026e95a498cd21008da6259053c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637930431751961803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=xgMIrKqFL4Vgw%2FT2EelyK%2BCv%2BWayJ7naUUklDgVe70o%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FALUEL5ATM42QHA6TDRKGYO3VTKLWDANCNFSM527YQ72Q&data=05%7C01%7Ceyast%40microsoft.com%7C27fd3026e95a498cd21008da6259053c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637930431751961803%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LNjyjIhnefmazXwgUSSNhmeesHiX36BcQyjfyBzQGxs%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>

zhumingpassional commented 2 years ago

I prefer writing a TestDataProcessor class, and put all test data processors there. This can reduce the number of test_xxx.py

eyast commented 2 years ago

So you want to setup some time and discuss ? I don't see the premise of having less test files or just one class. I don't mind, but I don't think we should rework

zhumingpassional commented 2 years ago

OK. i will merge it.