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

Automatically update eurusd.csv if required + introduction of unit tests #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quantx-heiko
Copy link
Contributor

Hi @laroche,
I don't like the necessity to update the eurusd.csv file manually if it is outdated, so I adjusted the code to automatically download a current version if it is required depending on the transaction history data.
Additionally I redesigned the code to a more class based approached which allows to implement unit tests. Some functionality has been moved to a TastytradeHelper class. I started implementing a couple of unit tests for its methods.

From my point of view that make future code changes much more easier, if developers can check if unit tests are still running without errors. I'd love to see unit tests in the future which check that certain transaction history inputs deliver a certain tax data output.

Please review my code changes.

@quantx-heiko
Copy link
Contributor Author

The unit tests can be executed either directly from VSCode "Test" tool section on the left hand side or directly from the Terminal using the following command:

python -m unittest discover -v -s ./tests -p "*_test.py"

@laroche
Copy link
Owner

laroche commented Oct 7, 2024

Current code should already download the eurusd data if no local file exists, but normally I don't want
to depend on a working Internet connection and I often run in a loop to generate output tax data for all years.

I'll look at the code and check to merge it in.

best regards,
thanks for your new code,

Florian La Roche

@quantx-heiko
Copy link
Contributor Author

It still will work without internet connection as long as the eurusd.csv file got the conversion rates for the required transaction dates available. I just didn't like the fact that the user needs to take special care of the conversion rate file if it is available but outdated.

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.

2 participants