-
Notifications
You must be signed in to change notification settings - Fork 68
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
revert and deprecate async_epidata (+ Remove usage of PHP alias in the Python client) #1295
revert and deprecate async_epidata (+ Remove usage of PHP alias in the Python client) #1295
Conversation
@rzats can you make this failing test work? I think it might be pretty easily fixable if you use |
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.
Tested filterwarnings
in a separate python file and it only printed the warning once 👍
responses = [i[0]["epidata"] for i in test_output] | ||
# check response is same as standard covidcast call (minus fields omitted by the api.php endpoint), | ||
# using 24 calls to test batch sizing | ||
ignore_fields = ["source", "time_type", "geo_type"] |
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.
instead of re-defining, use existing list:
ignore_fields = ["source", "time_type", "geo_type"] | |
ignore_fields = CovidcastTestRow._api_row_compatibility_ignore_fields |
[{k: v for k, v in row.items() if k not in ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[0]))["epidata"]], | ||
[{k: v for k, v in row.items() if k not in ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[1]))["epidata"]], |
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.
this is slightly more concise...
[{k: v for k, v in row.items() if k not in ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[0]))["epidata"]], | |
[{k: v for k, v in row.items() if k not in ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[1]))["epidata"]], | |
[{k: row[k] for k in row.keys() - ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[0]))["epidata"]], | |
[{k: row[k] for k in row.keys() - ignore_fields} for row in Epidata.covidcast(**self.params_from_row(rows[1]))["epidata"]], |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
awesome!!! thank you @dshemetov ! |
whoops, i wasnt paying attention on the command line and my commit message got slightly mangled, it should read:
"revert
async_epidata()
to previous behavior, and mark deprecated (filterwarnings()
not fully tested)"to be added to #1288
i did not do much testing around the
warnings.filterwarnings()
call, but it is intended to print the deprecation warning for the method only once (on its first use), no matter howasync_epidata()
is called.