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

Fetching Senate / HOR financial disclosures #6674

Closed

Conversation

mmistroni
Copy link
Contributor

Pull Request Template for OpenBB Developers

  1. Title:

    • Senate/HOR financial disclosures
  2. Why? (1-3 sentences or a bullet point list):

    • Currently only available via FMP (paid susb) and quiverquants. This is a joint effort between @deeleeramone (senate) and myself (hor).
    • It's still a WIP - i am not skilled in asyncio - and somehow my ipynb retrieves more disclosures (480) than the version i ran in IDEA
  3. What? (1-3 sentences or a bullet point list):

    • Updated government-us provider to expose senate and house of reprsentative disclosures
  4. Impact (1-2 sentences or a bullet point list):

    • N.A. wont affect any existing endpoints
  5. Testing Done:

    • Ran the notebook, and ran test_hor_helpers.py. Somehow the pytest.http does not work due to the presence
      of a ZIP file which is extracted on the fly, atm i dont know how to get around it
    • Have provided a asample XML file for disclosures, a sample PDF for a member disclosue and a jupyter notebook that runs the api
  6. Reviewer Notes (optional):

    • Any specific focus areas for review?
      QueryParams. not really sure what should be the driver for this data. for senate i thought number of reports as the api is quick enough to retrieve all of them at once
      for HOR, i have made driven by year as the HOR URLS has data by years. FMP exposes also a 'query by member' but this will require a new implemetnation similar to what @deeleeramone has done for senate
  7. Any other information (optional)
    Please advise on potential improvement soi can try to work on them myself

@deeleeramone
Copy link
Contributor

Hi @mmistroni, thanks for the PR and I appreciate the efforts, but I will need to kindly decline.

The code I shared with you was not intended as a solution appropriate for submitting as a PR. The code would be significantly different if I were to submit as a PR. While it's fine for personal use as a notebook, It is not a viable solution for a production environment and will be unreliable in operation. After 1 or 2 queries the IP address is flagged and banned for the day, so this approach is clearly not going to work at scale.

There are some minimum requirements when considering a function that serves this data:

  • Queryable by stock ticker.
  • Queryable by date range.
  • Queryable by representative.
  • The standard model -> provider model -> fetcher pattern needs to be followed.
  • Users won't know - or likely care - about the difference between HOR and Senate, a representatives endpoint should gracefully handle both with none or very little user intervention.
  • Provider models need to map to the router, there is no endpoint created which means there is no function.

Some general notes:

  • There are too many files.
  • There are files that are unrelated to the PR.
  • Notebook files are for Examples only, they have no purpose within the library code.
  • Tests should not need to stray at all from the established pattern throughout all unit and integration tests.

@mmistroni
Copy link
Contributor Author

mmistroni commented Sep 23, 2024 via email

@mmistroni
Copy link
Contributor Author

@deeleeramone i will carry on working on this.. i might need some assistance..can i post here rather than pinging you directly on Discord?
i m keen on completing this... kr

@markqiu
Copy link

markqiu commented Oct 10, 2024

Why don't we just add this to fmp provider?

@mmistroni
Copy link
Contributor Author

mmistroni commented Oct 10, 2024 via email

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.

3 participants