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

Better missing security reporting #43

Merged
merged 5 commits into from Jan 16, 2023

Conversation

thehilll
Copy link
Contributor

If get_ticker_info_from_id finds a missing security_id try to use a security_list in the ofx itself to print out useful information.

Fidelity ofx files appear to have a full security list embedded, and in most (all?) cases this is all that is needed to update fund_info.py. All that this does is print out a list of the symbols (as is already done) and the name provided in the ofx.

If security_list is missing from the ofx the behavior should be no worse than today...print a list of missing symbols.

Ware Adams added 2 commits January 14, 2023 14:34
…y to use self.ofx.security_list

to report more useful information.  At least in Fidelity ofx files both the symbol and security name
are present, so in most cases the required additions to fund_info.py are already in the file.
@thehilll
Copy link
Contributor Author

Example of the change, old:

Error: fund info not found for XYZ
List of securities without fund info: ['XYZ']

New:

Error: fund info not found for XYZ
List of securities without fund info:
XYZ: THE XYZ COMPANY

@redstreet
Copy link
Owner

Nice upgrade, thanks for the PR! Happy to merge it, would you mind addressing the linting error?

@thehilll
Copy link
Contributor Author

Sorry, I wasn't familiar with the f string notation. Should have looked it up

2. Key the extracted securities dict by CUSIP (not symbol) which matches what is found
     in the transaction entries (not for bonds symbol generally matches CUSIP)
3. Values of dict are now a tuple of (symbol, cusip, name) which for stocks should be all
     that is needed for fund_info.py
@thehilll
Copy link
Contributor Author

After thinking about this some more I realized it was probably only correct in the case of a bond where CUSIP matches symbol in the OFX (which was the example I ran into today). The OFX securities list does include CUSIP, symbol and name, so I changed this to return all three in a tuple that should match what is needed for fund_info.py (for shares...I don't think CUSIPS can be bean count symbols b/c they start with numbers, or many do).

@redstreet
Copy link
Owner

redstreet commented Jan 15, 2023

Linter failed again 🙂. Just a minor thing: the f needs to be removed. Sorry this is annoying. I think GitHub will let you run the linter yourself on future PRs.

@thehilll
Copy link
Contributor Author

Ugh, unbelievable...same issue but one line away. Sorry.

Unless I'm looking in the wrong place I think it would let me run it had I merged something before, but it says "First-time contributors need a maintainer to approve running workflows"

@redstreet redstreet merged commit 41128b9 into redstreet:main Jan 16, 2023
@redstreet
Copy link
Owner

Heh, all good now, and further PRs will let you run the linter. Thanks!

scanta2 pushed a commit to scanta2/beancount_reds_importers that referenced this pull request Jan 5, 2024
* if get_ticker_info_from_id finds an id not present in fund_info.py try to use self.ofx.security_list
to report more useful information.  At least in Fidelity ofx files both the symbol and security name
are present, so in most cases the required additions to fund_info.py are already in the file.

* Slightly better reportings...print a summary line before list of securities

* remove unused f string

* 1. Comments to explain what is going on
2. Key the extracted securities dict by CUSIP (not symbol) which matches what is found
     in the transaction entries (not for bonds symbol generally matches CUSIP)
3. Values of dict are now a tuple of (symbol, cusip, name) which for stocks should be all
     that is needed for fund_info.py

* Remove another unnecessary f string

Co-authored-by: Ware Adams <ware@rwa.washdcmail-intranet.lan>
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