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 TD Ameritrade CSV importer #54

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

reedlaw
Copy link

@reedlaw reedlaw commented Mar 28, 2023

After getting some help with this issue, I think this importer is good enough to share.

@redstreet
Copy link
Owner

Great, thanks for sharing! Left some comments. Also, would you be able to share an anonymized .csv file that can be used for testing?

@reedlaw
Copy link
Author

reedlaw commented Mar 28, 2023

I've added an example and script following the Fidelity CSV pattern. I wasn't able to run it because I'm not sure how to require the local Python package instead of the system version. I think it involves creating a virtual environment but I'm not proficient enough in Python to know how to do it.

@redstreet
Copy link
Owner

That's great. The example is sufficient. I'll add the test code to run it, which I need to unify for all importers anyway.

If you don't mind addressing the other comments above, I'll be happy to merge in this PR. Thanks!

@redstreet redstreet force-pushed the main branch 2 times, most recently from dbd86a4 to 9a9d64a Compare May 6, 2023 01:18
@redstreet redstreet force-pushed the main branch 2 times, most recently from fd4372d to c6d82e9 Compare September 9, 2023 20:08
@redstreet
Copy link
Owner

Hi @reedlaw, just checking to see if you are still inclined to complete this PR and merge? Thanks!

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