matthewgilbert / blp

Pythonic interface for Bloomberg Open API
Apache License 2.0
112 stars 24 forks source link

add support beqs #26

Closed avantgardeam closed 1 year ago

avantgardeam commented 1 year ago

Hello!

Last week I opened an issue (#25) from my personal account. The issue relates to the support for beqs, and I believe I have addressed it in this pull request.

However, this is my first pull request ever, so I'm not entirely sure if everything is in order. Any feedback on that matter would be great!

Finally, if you think it's good to go, I can start work on some documentation for the new functionality.

Thanks!

matthewgilbert commented 1 year ago

This looks good, I pushed one commit to add some slight code formatting since I try to follow black code formatting conventions https://black.readthedocs.io/en/stable/

matthewgilbert commented 1 year ago

Just wanted to confirm you have ran this and it works as expected? I don't currently have access to a terminal so I'm unable to test the feature.

avantgardeam commented 1 year ago

Just wanted to confirm you have ran this and it works as expected?

I have tested a few examples, and everything seems to be functioning well. However, I haven't conducted a systematic test. If you have specific tests you'd like me to run, feel free to share them, and I'll be happy to execute them.

avantgardeam commented 1 year ago

Hello, @matthewgilbert, following your advice, we are now preserving the 'Ticker' column rather than discarding it. Also, given EQS's way of displaying metrics and ranks side by side, I've taken out the part of the code that sorts columns alphabetically. Do you believe any further adjustments are necessary before merging?

P.S. I have tested this with a private screen using a backdate and it functioned as expected. Additionally, it performed well on a public screen once the timeout was adjusted.

matthewgilbert commented 1 year ago

Great, merged! Thanks for contributing.