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

[BUG] Try to convert Currency when eod and statement are same currency #150

Closed
adahan opened this issue Jul 24, 2024 · 9 comments
Closed

Comments

@adahan
Copy link

adahan commented Jul 24, 2024

What's the feature that should be improved?
Downloading a statement will try to convert the statements to market data currency. But when those currency are equal this is not needed resulting in additional waste of call to try to download fx

Describe how you would like the feature improved
Toolkit should check if those currency are equal and try to convert only if needed. Idealy fx should be cached so its not redownloaded everytime

Possibly describe the ideal way to improve this
Check if currency are different before converting

Additional information
Check could be done line 113 in helper.py on determine currencies function

@JerBouma
Copy link
Owner

Any ideas on how to facilitate the caching? I'll fix this somewhere in the upcoming weeks.

JerBouma added a commit that referenced this issue Aug 5, 2024
@JerBouma
Copy link
Owner

JerBouma commented Aug 5, 2024

Issue has been resolved! It will now only collect data on currency tickers that hold actual exchange rate data (and is not simply 1).

@JerBouma JerBouma closed this as completed Aug 5, 2024
@draj05
Copy link

draj05 commented Aug 5, 2024

I have a bad feeling you have unintentionally broke the data download. I cannot download almost any data now, which I could few hours back and this error apears. Or the change is so recent, that the data are not yet available for the exchange rate conversion. But all the downloads show critical error on exchange data download, which I think was not present previously and I expect it to be added as new related to this issue. Or there is somewhere new input variable for the exchange rate setting I am missing.

Obtaining cashflow data: 100%|██████████| 1/1 [00:00<00:00, 9.62it/s]
Obtaining historical statistics: 100%|██████████| 1/1 [00:00<00:00, 9.58it/s]
Obtaining exchange data: 0it [00:00, ?it/s]

ValueError Traceback (most recent call last)
in <cell line: 4>()
2
3 # a Historical example
----> 4 companies.get_cash_flow_statement()

2 frames
/usr/local/lib/python3.10/dist-packages/financetoolkit/toolkit_controller.py in get_cash_flow_statement(self, overwrite, rounding, growth, lag, trailing, progress_bar)
3240
3241 if convert_currency:
-> 3242 self.get_exchange_rates(
3243 period="quarterly" if self._quarterly else "yearly",
3244 progress_bar=progress_bar

/usr/local/lib/python3.10/dist-packages/financetoolkit/toolkit_controller.py in get_exchange_rates(self, period, return_column, fill_nan, overwrite, rounding, show_ticker_seperation, progress_bar)
2632
2633 if self._daily_exchange_rate_data.empty or overwrite:
-> 2634 self._daily_exchange_rate_data, _ = _get_historical_data(
2635 tickers=currencies_to_collect_data_for,
2636 api_key=self._api_key,

/usr/local/lib/python3.10/dist-packages/financetoolkit/historical_model.py in get_historical_data(tickers, api_key, source, start, end, interval, return_column, risk_free_rate, include_dividends, progress_bar, fill_nan, divide_ohlc_by, rounding, sleep_timer, show_ticker_seperation, show_errors, tqdm_message)
236
237 if not historical_data_dict:
--> 238 raise ValueError("No data found for the given tickers.")
239
240 historical_data = pd.concat(historical_data_dict).unstack(level=0)

ValueError: No data found for the given tickers.

@JerBouma
Copy link
Owner

JerBouma commented Aug 6, 2024

Oops, I'll have a look today. Can you share your code?

@JerBouma JerBouma reopened this Aug 6, 2024
@JerBouma
Copy link
Owner

JerBouma commented Aug 6, 2024

Hi @adahan, this issue should now be resolved. The issue was that in the case that no ticker was a different currency (e.g. TSLA, AAPL and AMZN) it would try to collect data with an empty ticker list. This has now been resolved to automatically fill an empty object which is not used further on.

Let me know if you still run into issues.

See 280f773

@JerBouma JerBouma closed this as completed Aug 6, 2024
@draj05
Copy link

draj05 commented Aug 6, 2024

Hi Jeroen, Jakub here again. I see you changed the code. But new error arises.

The code I am trying is this, but it is essentially same for all downloads.

companies = Toolkit(["NVDA", "TSLA"], api_key=API_KEY, start_date="2023-12-31", quarterly= True)
companies.get_cash_flow_statement()

The results is this

Obtaining cashflow data: 100%|██████████| 2/2 [00:00<00:00,  9.54it/s]
Obtaining historical statistics: 100%|██████████| 2/2 [00:00<00:00,  9.40it/s]
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
[<ipython-input-2-ec6a2ea1c4e1>](https://localhost:8080/#) in <cell line: 4>()
      2 
      3 # a Historical example
----> 4 companies.get_cash_flow_statement()

2 frames
[/usr/local/lib/python3.10/dist-packages/financetoolkit/toolkit_controller.py](https://localhost:8080/#) in get_cash_flow_statement(self, overwrite, rounding, growth, lag, trailing, progress_bar)
   3261 
   3262         if convert_currency:
-> 3263             self.get_exchange_rates(
   3264                 period="quarterly" if self._quarterly else "yearly",
   3265                 progress_bar=progress_bar

[/usr/local/lib/python3.10/dist-packages/financetoolkit/toolkit_controller.py](https://localhost:8080/#) in get_exchange_rates(self, period, return_column, fill_nan, overwrite, rounding, show_ticker_seperation, progress_bar)
   2754 
   2755             if len(self._currencies) == 1:
-> 2756                 return historical_data.xs(self._currencies[0], level=1, axis="columns")
   2757 
   2758             return historical_data

[/usr/local/lib/python3.10/dist-packages/pandas/core/generic.py](https://localhost:8080/#) in xs(self, key, axis, level, drop_level)
   4271         if level is not None:
   4272             if not isinstance(labels, MultiIndex):
-> 4273                 raise TypeError("Index must be a MultiIndex")
   4274             loc, new_ax = labels.get_loc_level(key, level=level, drop_level=drop_level)
   4275 

TypeError: Index must be a MultiIndex

I was able to bypass it by commenting the section where the return historical_data.xs(self._currencies[0], level=1, axis="columns") is because I do not need it. I did not have time to examine closer but I expect that when you have only 1 exhange rate, the dataframe is not Multiindex and thus raising the error. My guess is the condition should be > 1 but I am not sure to be honest.

@JerBouma
Copy link
Owner

JerBouma commented Aug 6, 2024

Seems to break because of quarterly=True, I am on it.

@JerBouma JerBouma reopened this Aug 6, 2024
@JerBouma
Copy link
Owner

JerBouma commented Aug 6, 2024

Ok, got it, upgrade to v1.9.5 and let's hope that's it! I had forgotten to test whether quarterly results also worked well. This is now fixed by creating a MultiIndex placeholder DataFrame (USDUSD=X with values of 1).

image

@draj05
Copy link

draj05 commented Aug 6, 2024

Works perfectly, thank you very much for fix and thanks for your work and the whole package, it's tremendous piece of work. Have a good day.

@JerBouma JerBouma closed this as completed Aug 6, 2024
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

No branches or pull requests

3 participants