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

Feature/companies house #4721

Merged
merged 135 commits into from
Aug 28, 2023

Conversation

kulbinderdio
Copy link
Contributor

Description

  • [​​Addition of new data source, UK Companies House. Now able to search, check details and download filings including accounts ] Summary of the change / bug fix.
  • Link # issue, if applicable.
  • [

image

] Screenshot of the feature or the bug before/after fix, if applicable. - [ Wanted the ability to investigate companies including their filed accounts] Relevant motivation and context. - [Required Companies House API Key ] List any dependencies that are required for this change.

How has this been tested?

  • Please describe the tests that you ran to verify your changes.
    Used several different names for search functionality, e.g. search Shell Oil
    Used the company number returned to load that company, e.g. load 06791769
    Used following commands to review company data: officers, signifcontrol, filings
    Used data returned from filings command to download actual document, e.g. filingdocument MzM3NTQzMzc0NGFkaXF6a2N4
    Can also look at individual company officers, etc by passing in company number parameter, e.g.
  • Provide instructions so we can reproduce.
  • Please also list any relevant details for your test configuration.
  • Make sure affected commands still run in terminal
  • [ Added new commands into SDK and checked] Ensure the SDK still works
  • Check any related reports

Checklist:

Others

  • [ x] I have performed a self-review of my own code.
  • [x ] I have commented my code, particularly in hard-to-understand areas.

@reviewpad reviewpad bot added the feat L Large T-Shirt size Feature label Apr 9, 2023
@kulbinderdio
Copy link
Contributor Author

Could the failures in my tests be due to the fact the new service requires a key?

@codecov
Copy link

codecov bot commented Apr 9, 2023

Codecov Report

Patch coverage: 27.27% and project coverage change: -0.29% ⚠️

Comparison is base (df9a154) 58.29% compared to head (d5c52bd) 58.00%.
Report is 350 commits behind head on develop.

❗ Current head d5c52bd differs from pull request most recent head 64032a6. Consider uploading reports for the commit 64032a6 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4721      +/-   ##
===========================================
- Coverage    58.29%   58.00%   -0.29%     
===========================================
  Files          588      591       +3     
  Lines        53666    53903     +237     
===========================================
- Hits         31283    31266      -17     
- Misses       22383    22637     +254     
Files Changed Coverage Δ
openbb_terminal/core/config/paths_helper.py 50.00% <ø> (ø)
...erminal/core/sdk/controllers/alt_sdk_controller.py 0.00% <0.00%> (ø)
...inal/core/sdk/controllers/crypto_sdk_controller.py 0.00% <ø> (ø)
...inal/core/sdk/controllers/stocks_sdk_controller.py 0.00% <ø> (ø)
openbb_terminal/core/sdk/models/alt_sdk_model.py 0.00% <0.00%> (ø)
...penbb_terminal/core/sdk/models/crypto_sdk_model.py 0.00% <0.00%> (ø)
...bb_terminal/core/sdk/models/portfolio_sdk_model.py 0.00% <0.00%> (ø)
...penbb_terminal/core/sdk/models/stocks_sdk_model.py 0.00% <ø> (ø)
openbb_terminal/core/sdk/sdk_helpers.py 37.69% <ø> (+0.76%) ⬆️
openbb_terminal/core/sdk/trailmap.py 92.68% <ø> (ø)
... and 26 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@reviewpad reviewpad bot added feat XL Extra Large feature and removed feat L Large T-Shirt size Feature labels Aug 9, 2023
@jmaslek
Copy link
Collaborator

jmaslek commented Aug 17, 2023

This looks awesome! Small missing detail in the i18n file. No description on the charges:

│     search             search for companies by name                                                                                                                                                         │
│     load               retrieve basic company details by company registration number                                                                                                                        │
│     officers           retrieve company officies                                                                                                                                                            │
│     signifcontrol      retrieve those with significant control over the company                                                                                                                             │
│     filings            retrieve list of documents filed with Companies House (Max of 100 at one time)                                                                                                       │
│     filingdocument     download filed document                                                                                                                                                              │
│     charges            alternative/companieshouse/charges       

@kulbinderdio
Copy link
Contributor Author

thanks James, just trying to get things closed out as I haven't had much time lately. Hopefully not too much more to do

cheers

@kulbinderdio
Copy link
Contributor Author

for the life of me I can't see what the issue is with the print_help test. Both Actual and Expected look identical. I suspect it might be something to do with hidden chars (life feed?) and have recreated the text file and still keep getting the same issue. Any ideas @jmaslek ?

@jmaslek jmaslek added this pull request to the merge queue Aug 28, 2023
Merged via the queue into OpenBB-finance:develop with commit db9d462 Aug 28, 2023
13 checks passed
the-praxs pushed a commit that referenced this pull request Sep 4, 2023
* Addition of UK Companies House data. Allows you to search for companies and inspect their details and download any filings including accounts

* reformatting of code

* reformatting

* reformat code

* reformat

* fix codespell issue

* updated for changes in way key are handled

* small reformatting

* added timeout for requests

* formatting

* ruff order fixes

* black reformatting

* merge issue - removed function call_openbb

* black format change

* Added new Companies House functionality to SDK

* formattng

* more Formatting (black)

* Ruff fix

* change return type for function

* forced type conversion

* return type fix

* correct error return type

* fix return type

* deleted companieshouse tests as they require API key

* Extra validation added for when data not exist from remote calls

* Added extra commands to en.yml

* spelling mistake correction

* fat finger trouble corrected

* Save documents with identifying names and allow documents to be viewed directly within OpenBB

* ruff updates

* black changes

* ruff change

* Addition of currently loaded company information in menu

* add entry in the API keys guide for Companies Hosue

* adds images to api keys guide

* adds section in SDK API Keys Guide for Companies House

* test file for companieshouse_model

* changed test data due to ruff line size limit

* resolve merge issues

* resolve merge issue

* doc strings examples

* added docstrings for companieshouse_model

* allow download_filking_document methid in view class to be called from SDK

* added docstring for download_filing_document SDK method

* correct Companies house key literal

* replace starnge . with ,

* add check_api_key decorator to each methopd in controller

* implement next and previous for looping through filings

* ruff changes

* correct tests

* companing filings now allows you specify category

* ruff line length issue resolved

* retrieves all filings  and  added charges command

* bug fix

* Added limit parameter to filings command, def :100

* fix ruff issue

* added get_charges to sdk and to test class

* resolved ruff line length issue

* add some view tests

* add some controller tests

* applying fix not merged to develop yet, column_keep_types=['Company Number']

* fix i18n description

* update test_print_help

* actually update test_print_help.txt

* Update en.yml - no colon

* rerorder imports - ruff failurre

* simple text change

* ruff fix

* help text change trying to fix pytest issue

---------

Co-authored-by: Danglewood <85772166+deeleeramone@users.noreply.github.com>
Co-authored-by: James Maslek <jmaslek11@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat XL Extra Large feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants