-
Notifications
You must be signed in to change notification settings - Fork 45
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
Support updated JSON format in Schwab Equity Award parser #467
Conversation
2fc6ba6
to
e7b281a
Compare
e7b281a
to
1ae9942
Compare
else: | ||
raise ParsingError( | ||
file, f'Parsing for action {row["action"]} is not implemented!' | ||
file, f"Parsing for action {row[names.action]} is not implemented!" | ||
) | ||
|
||
currency = "USD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: a few lines below (line 311+ in the new code/241+ in the old code - I can't add a comment there directly sadly) the _normalize_split()
function seems to assume that the data is about Alphabet shares. I think some check on the stock symbol might be prudent to avoid accidentally scaling some other stock's values according to the Alphabet split, in cases where the user happens to also use Schwab as broker for employer-issued equity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's right, that's for Alphabet's stock split. I am not sure if Schwab gives the same data format as parsed by this code for other accounts than the Google Equity Awards ones which can only have GOOG and GOOGL.
But I agree it'd be better to also check on the stock ticker name to be safe.
Not directly relevant to this PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equity awards accounts and this JSON format are not unique to GOOG/GOOGL stocks, but I think this JSON format may be unique to equity awards accounts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great actually to have some JSON/CSV file in resources with information about splits to have general support for them instead of hardcoding a specific one directly in the code.
To me it seems we have two parsers for the same thing, in schwab.py there's |
Fair question, but the new CSV file format that Google employees get on their Equity Awards Account appear to have a different format, not supported by this parser.
That doesn't seem to match
In other words, Schwab seems to export data in different formats for different types of accounts? |
I thought the CSV parser was for "normal" individual brokerage accounts, and the equity awards JSON parser was for equity awards accounts. |
No, that’s just the normal transaction parser. There are two parsers in schwab.py, the award parser is in the function ‘_read_schwab_awards’ |
Also note that the csv parser for Schwab (both of them) are being fixed in #459 |
Thanks for finding how to export more data! |
Schwab have now updated their website to:
This PR updates the Schwab Equity Award parser to work with this updated format (while still supporting the previous one).