alpha-xone / xbbg

An intuitive Bloomberg API
https://xbbg.readthedocs.io/
Apache License 2.0
244 stars 51 forks source link

Added functionality to custom config usage in bdib #54

Closed hceh closed 2 years ago

hceh commented 2 years ago

Issue: when using a ref argument in exch_info (from bdib) and a config arg for other markets not listed in exch.yml, the dataframe passed to the config argument was omitted in the recurring call for exch_info on line 164.

Solution: add additional if statement that if config argument is used, pass this to the recurring call in addition to the ref argument

hceh commented 2 years ago

Updated with flake linting to pass checks

codecov[bot] commented 2 years ago

Codecov Report

Merging #54 (e720246) into master (5a92549) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #54   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          415       415           
=========================================
  Hits           415       415           
Impacted Files Coverage Δ
xbbg/const.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5a92549...e720246. Read the comment docs.

alpha-xone commented 2 years ago

config is supposed to contain missing exchange info. ref is used to look up existing exchange info in exch.yml.

Don't see why both of them are needed at the same time.

hceh commented 2 years ago

This is the conclusion I came to, but if there is an alternate way to do this without the change above please let me know.

Example ticker = "RBREW DC Equity"

There is no mapping in exch_cfg.pkl for this ticker, nor for danish tickers, so a new config of market hours is required for Denmark - so add an EquityDC row into this file with the relevant times and pass the table as config argument. Adding a row for every single ticker in a universe isn't a suitable solution.

However, RBREW DC Equity is still not in the index of this file, so const.market_info is used to get relevant index key for the ticker, but DC is not in Equity_cfg.pkl. To get around this a ref must be used to map RBREW DC Equity to EquityDC to then be used for the time axis bounds in bdib.

In this instance a config and ref are being used, but if a ref exists in the kwargs of exch_info, the config is not used without the above fix. With the above, I can pass the following:

data = bdib('RBREW DC Equity', config=new_config_with_EquityDC, ref='EquityDC')

alpha-xone commented 2 years ago

Below also cover your case right? If yes can you change to below and I'll merge into the main branch - thanks:

exch_info(ticker=kwargs['ref'], **{k: v for k, v in kwargs if k != 'ref'})

This will avoid adding infinite if / else clauses.

hceh commented 2 years ago

That's far neater than my suggested, though it needed .items()

exch_info(ticker=kwargs['ref'], **{k: v for k, v in kwargs.items() if k != 'ref'})
alpha-xone commented 2 years ago

You're right - .items() is needed.