razorhash / pyfmpcloud

A python wrapper for the Financial Model Prep API for analysis of public companies
MIT License
49 stars 9 forks source link

Inconsistency inputting ticker in stock_time_series and company_valuation #5

Closed dabrown645 closed 4 years ago

dabrown645 commented 4 years ago

With company_valuation ticker is supplied as a keyword parameter and with stock_time_series it is a positional parameter. I think for ease of use it would be better to be consistent between the two use positional or keyword for both.

razorhash commented 4 years ago

Hi @dabrown645. could you confirm which function within the stock_time_series module do you see ticker supplied as a keyword argument? I just checked and in both the above modules, ticker is always a positional argument (1st argument), where applicable, except for batch_request_eod. This is to tackle the way the API is built. Did I understand you incorrectly here?

dabrown645 commented 4 years ago

After working on issue 4 this one looks to be more of a documentation issue than a code issue. In the native fmpcloud API for company valuation and stock time series the ticker/ticker list is positional vs key/value pair and that is the way company_valuation.py and stock_time_series.py implement. However, In the README all the Company Valuation examples show ticker being entered as a key/value pair which is okay with python but might be better as positional as it is in Stock Time Series. Either way I think the examples should be consistent as it implies the proper way they should be called. Your thoughts?

razorhash commented 4 years ago

Agreed. The documentation should be consistent in this case. Would you like to work on this or shall I assign it to me?

razorhash commented 4 years ago

See commit: 6c12331f031afa9b9e01a7af07cf21054bf4a323