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

Add tests #190

Merged
merged 20 commits into from Jul 12, 2022
Merged

Add tests #190

merged 20 commits into from Jul 12, 2022

Conversation

eyast
Copy link
Contributor

@eyast eyast commented Jul 8, 2022

I have added several tests:

  • Added testcase for Binance
  • Added testcase for clean_data()
  • added testcase for add_technical_indicator(both stockstats and TALib)

@eyast
Copy link
Contributor Author

eyast commented Jul 10, 2022

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

@eyast
Copy link
Contributor Author

eyast commented Jul 10, 2022

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
Copy link
Collaborator

zhumingpassional commented Jul 10, 2022

@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
Copy link
Contributor Author

eyast commented Jul 10, 2022 via email

@zhumingpassional
Copy link
Collaborator

zhumingpassional commented Jul 12, 2022

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

@eyast
Copy link
Contributor Author

eyast commented Jul 12, 2022

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
Copy link
Collaborator

OK. i will merge it.

@zhumingpassional zhumingpassional merged commit 425ac28 into AI4Finance-Foundation:master Jul 12, 2022
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

2 participants