Finnhub-Stock-API / finnhub-python

Finnhub Python API Client. Finnhub API provides institutional-grade financial data to investors, fintech startups and investment firms. We support real-time stock price, global fundamentals, global ETFs holdings and alternative data. https://finnhub.io/docs/api
https://finnhub.io/
Apache License 2.0
585 stars 101 forks source link

Enhancing & Fixing the Implementation #13

Closed bl4ckst0ne closed 4 years ago

bl4ckst0ne commented 4 years ago

Current Changes

Hey, I have looked at the code, and found several places in the implementation that could be enhanced / fixed:

  1. Inconsistent use of quotes (mixed single quotes as well as double quotes).
  2. Styling issues, such as imports order, line length, etc.
  3. Unnecessary functions (one liners, that are used once).
  4. Static methods that were declared as class functions.
  5. _request implementation.
  6. _handle_response does not handle csv format.
  7. The internal session is never closed.
  8. Formatting boolean parameters.

_request Implementation

As far as I'm concerned, the _request implementation has several things to be improved:

  1. _create_api_uri is redundant.
  2. _request_api is redundant, and _create_api_uri should be called from _request.
  3. Using data, and then removing it and setting it as params is messy.
  4. The parameter of the API token should be passed as a default argument inside session.params or session.headers (both of them are valid, according to the documentation).

_handle_response Implementation

_handle_response function will raise exception when the format of the output is csv. It can be fixed by checking the returned content type from the server.

Internal Session Object

The internal session object is never (but should be) closed. Therefore, we should add close function, and support for the Client as a context manger (by implementing __enter__ and __exit__).

Formatting Boolean Parameters

The current implementation tries to format the whole keyword arguments dictionary. In other words, the keyword arguments themselves are checked inside _str_to_bool (e.g, the data dictionary, instead of its content). Therefore, there is no conversion.

For example:

r1 = client.earnings_calendar(_from="2020-06-10", to="2020-06-30", symbol="", international=False)
r2 = client.earnings_calendar(_from="2020-06-10", to="2020-06-30", symbol="", international=True)
r3 = client.earnings_calendar(_from="2020-06-10", to="2020-06-30", symbol="", international="true")

After execution it, we can notice that r1 == r2, but r1 != r3 and r2 != r3. As a result, we can see there is no conversion.

Future Changes

In my opinion, the code can be enhanced even more. In general, I think it will be great if you add tests, that will check the API responses, as well as linters and formatting tools. Consider using:

Also, consider add documentation to the library.

nongdenchet commented 4 years ago

hey @bl4ckst0ne thank you for contributing, we will have a look 👍