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

Add BEQS functionality #79

Merged
merged 13 commits into from Oct 3, 2015
Merged

Add BEQS functionality #79

merged 13 commits into from Oct 3, 2015

Conversation

@csrvermaak
Copy link
Contributor

@csrvermaak csrvermaak commented Oct 2, 2015

Hi All

I've added the BEQS functionality, and committed the updates to my fork. I'm creating a pull request for you to review, and merge if happy.

Regards

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Thanks for the pull request. I will try to take a look. What make this a little difficult is something you clearly see by looking at the diff following 'Files changed' above: a lot of needlessly changed whitespace-only stuff. I think one may be able to do better by picking different CR vs CR/LF options, or something related. I would not know as I don't work on "that OS". So I may include the changes one by one (and filtering the files again) rather than accepting a pull request which brings is a lot of needless whitespace change.

R/beqs.R Outdated
##' Use PRIVATE for user-defined EQS screen.
##' Use GLOBAL for Bloomberg EQS screen.
##' @param Group An optional character string with the Screen folder name as defined in EQS
##' @param PiTDate A character string with the Point in Time Date of the screen to execute.

This comment has been minimized.

@eddelbuettel

eddelbuettel Oct 3, 2015
Member

Could 'Group' and 'PitDate' be lower-case arguments (which is more common) or does Bloomberg use uppercase?

This comment has been minimized.

@csrvermaak

csrvermaak Oct 3, 2015
Author Contributor

These are as defined in the blpapi developers guide 2.54

R/beqs.R Outdated
##' @author Whit Armstrong, Dirk Eddelbuettel
##' @examples
##' \dontrun{
##' beqs(screenName = "PRODUCTION(JALSH)",screenType ="PRIVATE",Group = "JALSH",PiTDate = "20021231",con = con)

This comment has been minimized.

@eddelbuettel

eddelbuettel Oct 3, 2015
Member

Is there a way for me to run a test of BEQS -- and I don't have a screen set up on my terminal.

This comment has been minimized.

@wmorgan85

wmorgan85 Oct 3, 2015
Contributor

There are some public screens you can use. screenType would be PUBLIC then the name of the search. I'll find you one of the common ones and post shortly.

This comment has been minimized.

@csrvermaak

csrvermaak Oct 3, 2015
Author Contributor

Thanks @wmorgan85 - but I'm busy updating the code to better handle the optional parameters, and will also add examples of every possible override:

res<-as.data.frame(beqs(screenName = "Frontier Market Stocks with 1 billion USD Market Caps"))

res<-as.data.frame(beqs(screenName = "Vehicle-Engine-Parts","GLOBAL"))

res<-as.data.frame(beqs(screenName = "Vehicle-Engine-Parts","GLOBAL","GERMAN"))

res<-as.data.frame(beqs(screenName = "Vehicle-Engine-Parts","GLOBAL","GERMAN","GENERAL"))

res<-as.data.frame(beqs(screenName = "Vehicle-Engine-Parts","GLOBAL","ENGLISH","GENERAL","20150930"))

Should do a pull request shortly.

This comment has been minimized.

@csrvermaak

csrvermaak Oct 3, 2015
Author Contributor

@wmorgan85 - sorry, could you please post one of the common ones? The one I have doesn't work, and not at the office at the moment to can't log in to BBG.

screenName = "Frontier Market Stocks with 1 billion USD Market Caps" - seems to be invalid

This comment has been minimized.

@wmorgan85

wmorgan85 Oct 3, 2015
Contributor

You can use screen 'Core Capital Ratios' which is part of the 'General' group. All the samples are in folders so when working with those you have to pass the folder name. Most EQS users don't organise their own screens into folders though so when retrieving a personal screen, group would most likely be null.

This comment has been minimized.

@csrvermaak

csrvermaak Oct 3, 2015
Author Contributor

Hmmm - something doesn't give. I use these, from the Spreadsheet EQS downloader (the one you sent me a while back)

Screen Name: "Core Capital Ratios"
Screen Type: "GLOBAL"
Screen Group: "General"

But I get this message:

BeqsResponse = {
responseError = {
source = " apieqs [nid:1158]"
code = 4080
category = "NOT_AVAILABLE"
message = "eqssvc: PIT_SCREEN_INVALID: Failed to validate point in time screen [nid:1158]"
}
}

time screen [nid:1124]"
}
}

This comment has been minimized.

@csrvermaak

csrvermaak Oct 3, 2015
Author Contributor

Ah - ok, the Core Capital Ratios screen must have non PiT variables, and doesn't work when given a PiT date.

Do you have a "General" screen for which I can test a PiT call?

This comment has been minimized.

@wmorgan85

wmorgan85 Oct 3, 2015
Contributor

Ok in this case it's because the screen is not supported historically. It will work without PiTDate but one of the columns is not available point in time. Try 'Global Oil Companies YTD Return' instead as a sample screen as it supports PIT.

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

Thanks @eddelbuettel , apologies for the whitespace - I'll check if it is something on my side.

Something to note: in beqs.r, when roxygen2::roxygenise() converts to .Rd, my output looks different in the helpfile (?besq from R prompt) - I have no whitespace between @param and description in the arguments. Not sure if this is Windows related - any ideas?

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

@csrvermaak Not sure, could be a roxygen or Windows issue.

Also, looks like because I added things the patch no longer cleanly merges. That is probably just because of ChangeLog which you could keep outside the PR (ie try to remove your change, merge with where master is now, commit to update the PR). We get the ChangeLog -- I am old-school on those and like them, but as a single file they make multiple concurrent changes more difficult.

@wmorgan85 Thanks for the help with the screens. It would be great to have one or two example calls for the Examples: section of the help page.

Add languageID parameter
Add working examples given existing BEQS screens
@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

@eddelbuettel - I think you're right, it should be only the ChangeLog that has a conflict. Could you accept all changes except the ChangeLog? I'm a bit inexperienced on GitHub to remove the change on ChangeLog retroactively. I'm about to do a pull request for updated code with working examples that should run seamlessly on your box.

@wmorgan85 Thanks for your help Morgan!

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

@csrvermaak I could. It is nice, though, to get a pull request accepted. So if you want to, you can make it easier for me on your end. What I would do is to

  • updated Rblp master (you seem to be working in your master rather than a branch, so the simplest would just be a new checkout into a directory, say, git/Rblp-upstream. Or into svn if you must.
  • keep your ChangeLog entry (in the editor, say)
  • copy our ChangeLog file over, then reapply your bit -- this should now diff cleanly again
  • commit in your repo and push, this will automagically update the pull request and should render it mergeable
  • it would be really nice if we got the changes to files
    • NAMESPACE down to one line
    • maybe have no change in rblapi.Rprof: it is just one line which does NOT apply to us as we do not have Rblpapi/trunk
    • ditto for the vignette: diff just be the change

Come to think about it I think I do blame your svn use for the excessive whitespace and changeset.

If the above is to hairy, I can checkout your repo and changes but they may end up be attributed to me (at GitHub) as my commits.

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

@eddelbuettel , thank you for the comments - much appreciated. I will do what you suggested, good exercise :-)

As an aside... I'm busy installing Git, and the Git installer confirms your blame for whitespace etc.

image

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

:-)

That is the spirit. Also, in all seriousness, maybe try git? It is the weekend, and bandwidth not withstanding now that we've come this far I'd be happy to help you set it up (if need be over quick Google Hangout session). I suspect it may make your life easier. Download git for Windows, point RStudio to it (and then restart RStudio, seemingly). Or maybe we leave that for the next round...

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

Thank you very much for your kind offer. I am seriously considering git, it is great. Two reasons for perhaps being a bit wary, would appreciate your comments on these, if you have an opinion:

  1. Is it possible to run both git and svn simultaneously (in Windows :-) )
  2. I wouldn't want all of my code to be visible - svn is free, and I keep my repository on a local server. Can I create a local git server, or is git purely cloud based, and I would need to upgrade for a private repo?

Thanks for your comments.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Regarding your two questions:

  1. Of course it is. Just like zip and tar+gzip (and umpteen other compressors like xz or even 7zip for you windows head). Just keep the repos separated or otherwise you may be out of sync between your svn and your git.
  2. You confuse a few things. What is on your disk is on your disk only. Git does not need a local server. And what you push to github is public either way--whether you use svn (to git) or git (directly) to push.
    Nobody can see your repositories unless you want them too. There is no 'git will automatically make it public'.
@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

Thank you - I will most probably move to git, just need a bit of time to digest and read up a bit more.

I believe I have just updated the pull request, with a new
NAMESPACE (as per master, with one line added)
rblpapi.proj (as per master)
\vignettes\rblpapi-intro.Rmd (as per master, with beqs line added)

I hope you can now merge?

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

It seems that the NAMESPACE file is now correct for merging. Something not good with vignette... Give me a sec :-)

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Yes, NAMESPACE looks perfect, the proj file is gone. Two steps in the right direction.

We still to make sure ChangeLog is only one paragraph, and ditto for the vignette. Worst case we exclude these for now. I can commit them by hand. The core part of your work and PR are the C++ and R file.

@@ -83,6 +83,9 @@ Historical data (at a daily granularity) can be retrieved with `bdh()`:

```{r, eval=FALSE}
bdh("SPY US Equity", c("PX_LAST", "VOLUME"), start.date=Sys.Date()-31)
```### beqs: Bloomberg EQS data can be retrieved with `beqs()`:

This comment has been minimized.

@eddelbuettel

eddelbuettel Oct 3, 2015
Member

We are missing a newline here.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Getting there.

But when I look at your repo it says

This branch is 7 commits ahead, 11 commits behind Rblp:master.

The 11 commit is a little imperfect, and the likely reason why there is still a (phantom) commit in Changelog.

Please replace your version of ChangeLog with the current one. You can even download from the browser via 'raw'.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Also, this is now seven^eight commits, when it should be one.

I do not usually insist on squashing multiple commits in a pull request but I am learning towards doing that.

So if you go to your settings you should be able to add me as committer to your repo, and I could then try to hotfix this into a single (clean !!) commit. Shall we try that?

And next time we start with and shoot svn behind the barn....

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Looks like you are conflicting again. I'll probably start my day proper here and get away from the computer at some point. We can pick things up later.

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

Hehe - yes, please @eddelbuettel - I'm not sure where the problem lies, but happy to shoot svn behind the barn :-) It might also be the difference between Windows and Linux (I assume you're on Linux?)

http://www.cs.toronto.edu/~krueger/csc209h/tut/line-endings.html

I will add you as a committer to my repo.

Thank you for your patience.

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Well as I see it there were two or three issues:

  • you took a snapshot with svn, and our repo moved on
  • in git keeping your branch current with our master is trivial
  • line endings are killing us, they are a frequent source of trouble for svn but not git
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Ok, I had it rebased but then need more work on vignette and changelog which seems to have undone that. Let me have one more pass on this.

eddelbuettel added a commit that referenced this pull request Oct 3, 2015
Add BEQS functionality
@eddelbuettel eddelbuettel merged commit 4bd1df7 into Rblp:master Oct 3, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

Well whatever. At least the PR is clean now, the commit history is a bit of a mess. Oh well.

(You can remove me again from your repo.)

@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Oct 3, 2015

And just to be very, very plain: You got lucky that I had some time this morning. I should really be working on something. Leave the svn repository as a reminder of things past, but learn git now.

The next PR that looks like this will be rejected.

@csrvermaak
Copy link
Contributor Author

@csrvermaak csrvermaak commented Oct 3, 2015

@eddelbuettel , thank you for your help in getting the PR merged. I appreciate your guidance, and will move to git. Hopefully the code is useful enough to justify the syntactic gymnastics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.