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

SECF() implementation #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

SECF() implementation #50

wants to merge 2 commits into from

Conversation

bverh
Copy link

@bverh bverh commented Oct 16, 2018

Closes #49 - all tests added / passed

>>> import pdblp
>>> con = pdblp.BCon(port=8194, timeout=5000)
>>> con.start()
>>> con.secf("IBM")
                        ticker                                        description
0                IBM US EQUITY        International Business Machines Corp (U.S.)
1                IBM UN EQUITY    International Business Machines Corp (New York)
2                IBM LN EQUITY      International Business Machines Corp (London)
3  IBM US 01/17/20 C145 EQUITY              January 20 Calls on IBM US Strike 145
4                IBM GR EQUITY     International Business Machines Corp (Germany)
5                IBM FP EQUITY  International Business Machines Corp (Euronext...
6  IBM US 10/19/18 C150 EQUITY              October 18 Calls on IBM US Strike 150
7                      IBM PFD               International Business Machines Corp
8                IBM CN EQUITY      International Business Machines Corp (Canada)

Returns a pd.DataFrame to be consistent with other functions.

Implementation of the terminal function 'SECF'  using the instruments service
raise RuntimeError("Expected a SERVICE_STATUS event but "
"received a %s" % ev_name)
if not opened:
logger.warning("Failed to open //blp/exrsvc")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be logger.warning("Failed to //blp/instruments")

ticker = r.getElementAsString("security")
ticker = ticker.replace('<', ' ').replace('>', '').upper()
descr = r.getElementAsString("description")
if not descr.endswith('(Multiple Matches)'):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the check for Multiple Matches doing? Is there a sample request you could add in the thread so I can see when this is used?

for msg in self._receive_events():
for r in msg.getElement("results").values():
ticker = r.getElementAsString("security")
ticker = ticker.replace('<', ' ').replace('>', '').upper()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to leave this line out. The general philosophy of pdblp is to provide a thin wrapper around blpapi and limit the data processing. If a user wants this they can do it easily enough afterwords using df.ticker.str.replace("<", "").str.replace(">", "").str.upper()

return data

data.append((ticker, descr))
return pd.DataFrame(data, columns=['ticker', 'description'])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the response messages look like

InstrumentListResponse = {
    results[] = {
        results = {
            security = "Some security"
            Description = "Blah blah"
        }
   ...
    }
}

I think that columns = ['security', 'description'] would be more appropriate

request = self.instrService.createRequest("instrumentListRequest")
request.set("query", query)
request.set("maxResults", max_results)
request.set("yellowKeyFilter", "YK_FILTER_%s" % yk_filter)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to be consistent with how parameters are set elsewhere in the library, this convenience string concatenation should not be done for the user, e.g. the user should pass YK_FILTER_EQTY and this line should be changed to request.set("yellowKeyFilter", yk_filter). Also while it does appear that YK_FILTER_NONE is supported but seems superfluous? I think it would be better stylistically to have yk_filter=None as a default parameter and then

if yk_filter:
    request.set("yellowKeyFilter", yk_filter)

@matthewgilbert
Copy link
Owner

Other than those minor things all looks good, thanks for the pull request.

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

Successfully merging this pull request may close these issues.

None yet

2 participants