Yandex.Disk Query runner #6598
Yandex.Disk Query runner #6598
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6598 +/- ##
==========================================
+ Coverage 61.89% 62.28% +0.38%
==========================================
Files 158 160 +2
Lines 12992 13114 +122
Branches 1774 1790 +16
==========================================
+ Hits 8042 8168 +126
+ Misses 4673 4663 -10
- Partials 277 283 +6
|
|
Re-running the frontend-e2e test, as it looks like it failed on the first attempt due to a GitHub infrastructure problem. 🤦 |
|
Oh hang on. It's failing on this:
Sounds like numpy might need to be added to the dependency list or similar? |
|
@justinclift looks strange, because numpy is the core dependency of pandas, and pandas is included in |
|
@guidopetri Do you have time to look at this one? |
|
@denisov-vlad I believe that there is no need to combine the dependencies of the main Redash and QR into one list. This can later lead to unnecessary dependencies |
redash/query_runner/__init__.py
Outdated
| if column_type == np.bool_: | ||
| redash_type = TYPE_BOOLEAN | ||
| elif column_type == np.inexact: | ||
| redash_type = TYPE_FLOAT | ||
| elif column_type == np.integer: | ||
| redash_type = TYPE_INTEGER | ||
| elif column_type in (np.datetime64, np.dtype("<M8[ns]")): | ||
| if df.empty: | ||
| redash_type = TYPE_DATETIME | ||
| elif len(df[column_name].head(1).astype(str).loc[0]) > 10: | ||
| redash_type = TYPE_DATETIME | ||
| else: | ||
| redash_type = TYPE_DATE | ||
| else: | ||
| redash_type = TYPE_STRING |
There was a problem hiding this comment.
Can we make this a mapping?
column_type_mappings = {
np.bool_: TYPE_BOOLEAN,
np.inexact: TYPE_FLOAT,
...
}
redash_type = column_type_mappings[column_type]There was a problem hiding this comment.
Oh I see, this was moved from elsewhere. I'd be fine keeping it as is then, but obviously better if it's made more readable :)
There was a problem hiding this comment.
Original code was also written by me :)
Thought about mapping here, but using extra if section for date/datetime prevented me from doing this.
|
|
||
|
|
||
| class TestYandexDisk(TestCase): | ||
| def test_xlsx(self): |
There was a problem hiding this comment.
These tests are great, but we only have ~30% coverage of the new query runner. Could we add some more tests to have >=90%?
|
Having the same issue as before with missed numpy package. Also could you please update Codecov report? |
|
The frontend e2e tests build the docker image without |
|
@guidopetri it was hard, but I've done it :) Could you review it again please? |
There was a problem hiding this comment.
This looks awesome! Thanks so much @denisov-vlad , merging now :)
What type of PR is this?
Description
Added query runner for Yandex.Disk, supports showing CSV, TSV, XLS(X) files
get_schemais used to show all available filesExample query:
How is this tested?
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)