We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
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
It would be helpful if you had a "Developer Note" in your README.md file, giving instructions on how to clone and run the package on a local computer - for example if there is an environment needed to be set up or what tests to run. #49
I would also suggest tidying up the README.md file a bit, for example including backticks around code to run in the "Installation" section of the README file running pytest tests/ --cov=traders_copilot_mzza_25 is not working for me. It looks like you may be missing this in your dependencies in your pyproject.toml file under [tool.poetry.group.dev.dependencies] #50 running pytest tests/ --cov=traders_copilot_mzza_25 is not working for me. It looks like you may be missing this in your dependencies in your pyproject.toml file under [tool.poetry.group.dev.dependencies]
Update: running poetry add --group dev pytest-cov, then poetry install, runs the test, then I had to update each test file to import from the correct path. For example in the test_generate_signals.py file, I updated the function import line to from traders_copilot_mzza_25.generate_signals import generate_signals. I updated all the files locally and was able to view the coverage report #51
Total coverage is 94% which is great! You could also include more coverage for the data.py function as it is only at 78% #47
You could add everyones github username using @ in the readme file to link, as well as email addresses (as suggested by the Metadata review heading, although i'm not sure if emails are required for us. #52
Update badges for test coverage, repo status, current package version, and Python versions supported. I believe this is a requirement for Milestone 4 though which is to be done this week anyway! #53
The ReadTheDocs documentation link is missing in the repository's About section and README. To improve accessibility, add the ReadTheDocs link (e.g., https://traders-copilot-mzza-25.readthedocs.io/en/latest/) in both places and include a Documentation Status badge in the README. Also, ensure the documentation covers all key functions, installation steps, and usage examples for completeness. #58
There is an inconsistency in function naming between the README and the actual implementation. The README references simulate_market_data, but the example uses generate_synthetic_data(). This discrepancy should be corrected to avoid confusion. #54
Currently, plot_signals() generates a visualization, but it lacks customization options for colors, labels, and grid settings. Allowing users to modify these parameters would improve usability, especially for traders who prefer specific styles. #55
The import instructions do not work in the terminal (python) I need to use traders_copilot_mzza_25.xxx to import any of the functions, otherwise, I will receive this error TypeError: 'module' object is not callable e.g. from traders_copilot_mzza_25.plot_signals import plot_signals instead of from traders_copilot_mzza_25 import plot_signals #56
The RSI calculation currently uses a simple moving average (SMA) for computing average gains and losses. Using an exponentially weighted moving average (EWMA) can provide smoother and more responsive RSI values, which is the standard approach in most trading platforms. Suggested change: avg_gain = gain.ewm(span=window, adjust=False).mean() avg_loss = loss.ewm(span=window, adjust=False).mean() #57 Suggested change: avg_gain = gain.ewm(span=window, adjust=False).mean() avg_loss = loss.ewm(span=window, adjust=False).mean()
The text was updated successfully, but these errors were encountered:
zananpech
abbyturi
MasonZhang-MZ
cherylziunzhao
No branches or pull requests
It would be helpful if you had a "Developer Note" in your README.md file, giving instructions on how to clone and run the package on a local computer - for example if there is an environment needed to be set up or what tests to run. #49
I would also suggest tidying up the README.md file a bit, for example including backticks around code to run in the "Installation" section of the README file running pytest tests/ --cov=traders_copilot_mzza_25 is not working for me. It looks like you may be missing this in your dependencies in your pyproject.toml file under [tool.poetry.group.dev.dependencies] #50
running pytest tests/ --cov=traders_copilot_mzza_25 is not working for me. It looks like you may be missing this in your dependencies in your pyproject.toml file under [tool.poetry.group.dev.dependencies]
Update: running poetry add --group dev pytest-cov, then poetry install, runs the test, then I had to update each test file to import from the correct path. For example in the test_generate_signals.py file, I updated the function import line to from traders_copilot_mzza_25.generate_signals import generate_signals. I updated all the files locally and was able to view the coverage report #51
Total coverage is 94% which is great! You could also include more coverage for the data.py function as it is only at 78% #47
You could add everyones github username using @ in the readme file to link, as well as email addresses (as suggested by the Metadata review heading, although i'm not sure if emails are required for us. #52
Update badges for test coverage, repo status, current package version, and Python versions supported. I believe this is a requirement for Milestone 4 though which is to be done this week anyway! #53
The ReadTheDocs documentation link is missing in the repository's About section and README. To improve accessibility, add the ReadTheDocs link (e.g., https://traders-copilot-mzza-25.readthedocs.io/en/latest/) in both places and include a Documentation Status badge in the README. Also, ensure the documentation covers all key functions, installation steps, and usage examples for completeness. #58
There is an inconsistency in function naming between the README and the actual implementation. The README references simulate_market_data, but the example uses generate_synthetic_data(). This discrepancy should be corrected to avoid confusion. #54
Currently, plot_signals() generates a visualization, but it lacks customization options for colors, labels, and grid settings. Allowing users to modify these parameters would improve usability, especially for traders who prefer specific styles. #55
The import instructions do not work in the terminal (python) I need to use traders_copilot_mzza_25.xxx to import any of the functions, otherwise, I will receive this error TypeError: 'module' object is not callable e.g. from traders_copilot_mzza_25.plot_signals import plot_signals instead of from traders_copilot_mzza_25 import plot_signals #56
The RSI calculation currently uses a simple moving average (SMA) for computing average gains and losses. Using an exponentially weighted moving average (EWMA) can provide smoother and more responsive RSI values, which is the standard approach in most trading platforms. Suggested change: avg_gain = gain.ewm(span=window, adjust=False).mean() avg_loss = loss.ewm(span=window, adjust=False).mean() #57
Suggested change:
avg_gain = gain.ewm(span=window, adjust=False).mean()
avg_loss = loss.ewm(span=window, adjust=False).mean()
The text was updated successfully, but these errors were encountered: