-
Notifications
You must be signed in to change notification settings - Fork 13
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
feature/dq_unit #379
feature/dq_unit #379
Conversation
…rosynergy into feature/dq_unit
Constructor removed - mock test still requires updating. |
Need to merge in PR381 or run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a few outstanding items.
macrosynergy/dataquery/api.py
Outdated
@classmethod | ||
def jpmaqs_indicators(cls, metrics, tickers): | ||
""" | ||
Functionality used to convert tickers into formal JPMaQS expressions. | ||
""" | ||
|
||
dq_tix = [] | ||
for metric in metrics: | ||
dq_tix += ["DB(JPMAQS," + tick + f",{metric})" for tick in tickers] | ||
|
||
return dq_tix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this one should be a staticmethod and not a classmethod. Class methods returns an instance of the class itself (Interface
), using return cls()
with some arguments, which instantiates the class .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ksteeds still need to convert to a static method or it needs to return an instance of the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converted to @staticmethod.
|
||
def test_authentication_error(self): | ||
with Interface( | ||
oauth=True, | ||
client_id="WRONG_CLIENT_ID", | ||
client_secret="NOT_A_SECRET" | ||
) as dq: | ||
with self.assertRaises(RuntimeError, msg="Authentication error - unable to access DataQuery:"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-add msg check, as it looks like it got removed by accident.
@@ -6,23 +6,24 @@ | |||
|
|||
|
|||
class TestDataQueryOAuth(unittest.TestCase): | |||
def test_connection(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add back in, as it shouldn't be removed
macrosynergy/dataquery/api.py
Outdated
results: dict = js["errors"][0] | ||
print(f"The corresponding code is: {results['code']}.") | ||
raise ConnectionError(results["description"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it more robust lets change it do:
try:
results: dict = js["info"]
except KeyError:
raise ConnectionError(f"DataQuery error response: {js}")
return int(results["code"]) == 200, results
that way we are independent of the structure of js
(the response).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
macrosynergy/dataquery/api.py
Outdated
results: dict = js["errors"][0] | ||
print(f"The corresponding code is: {results['code']}.") | ||
raise ConnectionError(results["description"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dq_error = f"DataQuery error response: {js}"
raise ConnectionError(dq_error)
Commencing Unit Test.