You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/01/28 10:26:07 UTC
[GitHub] [superset] TColl opened a new pull request #12806: refactor: vectorise big integer to string type conversion
TColl opened a new pull request #12806:
URL: https://github.com/apache/superset/pull/12806
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Converting big integers too strings was previously done manually whilst iterating over every value in the DataFrame.
New version makes use of pandas [DataFrame.applymap()](https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.applymap.html). This results in a 3x speedup on a local DataFrame of shape (20000, 2).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
N/A
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
TBD
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (ba92912) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 66.96% -0.05%
==========================================
Files 1022 489 -533
Lines 50102 28712 -21390
Branches 5191 0 -5191
==========================================
- Hits 33574 19228 -14346
+ Misses 16397 9484 -6913
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.96% <100.00%> (+2.88%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| ... and [571 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...ba92912](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (32a8c75) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `8.16%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 58.84% -8.17%
==========================================
Files 1022 964 -58
Lines 50102 47325 -2777
Branches 5191 4435 -756
==========================================
- Hits 33574 27849 -5725
- Misses 16397 19476 +3079
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.85% <ø> (-0.07%)` | :arrow_down: |
| javascript | `?` | |
| python | `63.38% <100.00%> (-0.70%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [424 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...32a8c75](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl removed a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl removed a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768955855
Re. testing; this is my first contribution to this repo and I'm not clear on preferred docstring style. If someone lets me know what you'd like to see I'll happily add docstrings and doctests to cover this refactor!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773885788
Let's keep this PR open for a couple of more days and see if people have ideas. The original code had a `TODO: refactor this`, so I'd assume the original author didn't like manual loops, either.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (d2ec8c5) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.35%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.36% +0.35%
==========================================
Files 1022 489 -533
Lines 50102 28717 -21385
Branches 5191 0 -5191
==========================================
- Hits 33574 19346 -14228
+ Misses 16397 9371 -7026
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.36% <85.71%> (+3.28%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <85.71%> (-8.34%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| ... and [569 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...d2ec8c5](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl removed a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl removed a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773988790
I've had another look at this, and dropping pandas `DataFrame.to_dict()` in favour of the underlying operation means we can do the integer conversion during the same single loop over all records in the dataframe, rather than looping over the whole dataframe twice to get to where we need to end up.
At the risk of making myself look stupid, this seems to result in a 2x speedup on my local tests this time round, but I'd appreciate a second pair of eyes on this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `5.08%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 61.92% -5.09%
==========================================
Files 1022 969 -53
Lines 50102 46031 -4071
Branches 5191 4485 -706
==========================================
- Hits 33574 28505 -5069
- Misses 16397 17526 +1129
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.97% <ø> (+2.04%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.32% <83.33%> (+3.24%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [444 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (350c4b5) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `3.61%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 63.39% -3.62%
==========================================
Files 1022 488 -534
Lines 50102 30138 -19964
Branches 5191 0 -5191
==========================================
- Hits 33574 19106 -14468
+ Misses 16397 11032 -5365
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.39% <100.00%> (-0.69%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
| ... and [545 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...350c4b5](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (e426840) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.24%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.25% +0.24%
==========================================
Files 1022 489 -533
Lines 50102 28685 -21417
Branches 5191 0 -5191
==========================================
- Hits 33574 19292 -14282
+ Misses 16397 9393 -7004
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.25% <100.00%> (+3.17%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `93.42% <0.00%> (-2.64%)` | :arrow_down: |
| ... and [570 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...e426840](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud edited a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-770099433
I'm a huge fan of performance optimization PRs so I did some benchmarking and added one of my own optimizations:
https://colab.research.google.com/drive/1M6wrvyIBIs079IC8RUGCT5I5zOxwOZ33#scrollTo=Fcie22gnZHqr
Based on my tests, the original method is actually the fastest (the result is consistent on both my local machine and Google Collab). Could you share how you tested the 3x performance gain with `Series.map`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.31%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.32% +0.31%
==========================================
Files 1022 489 -533
Lines 50102 28716 -21386
Branches 5191 0 -5191
==========================================
- Hits 33574 19333 -14241
+ Misses 16397 9383 -7014
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.32% <83.33%> (+3.24%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/examples/utils.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdXRpbHMucHk=) | `32.00% <0.00%> (-1.34%)` | :arrow_down: |
| ... and [561 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `4.91%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 62.09% -4.92%
==========================================
Files 1022 969 -53
Lines 50102 46031 -4071
Branches 5191 4485 -706
==========================================
- Hits 33574 28582 -4992
- Misses 16397 17449 +1052
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.97% <ø> (+2.04%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.59% <83.33%> (+3.50%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [442 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.24%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.25% +0.24%
==========================================
Files 1022 489 -533
Lines 50102 28688 -21414
Branches 5191 0 -5191
==========================================
- Hits 33574 19294 -14280
+ Misses 16397 9394 -7003
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.25% <83.33%> (+3.17%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `93.42% <0.00%> (-2.64%)` | :arrow_down: |
| ... and [570 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `5.22%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 61.78% -5.23%
==========================================
Files 1022 969 -53
Lines 50102 46031 -4071
Branches 5191 4485 -706
==========================================
- Hits 33574 28441 -5133
- Misses 16397 17590 +1193
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.60% <ø> (+1.67%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.32% <83.33%> (+3.24%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [444 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774456890
I've added the check/warning in and one final tweak (though we're really into diminishing returns at this point). With the latest code I'm seeing 2x speedup on large dataframes and 6-7x speedup on smaller ones - [benchmarking notebook](https://colab.research.google.com/drive/1_b2KKFrscNsbzceocxNpoNHGTS-IHbSL?usp=sharing). I'm surprised you're not seeing similar performance improvements; would you mind sharing how you've tested it?
I think this is finally good to go - thanks for bearing with me after the false start!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `0.09%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 66.91% -0.10%
==========================================
Files 1022 489 -533
Lines 50102 28688 -21414
Branches 5191 0 -5191
==========================================
- Hits 33574 19197 -14377
+ Misses 16397 9491 -6906
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.91% <83.33%> (+2.83%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| ... and [579 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768955855
Re. testing; this is my first contribution to this repo and I'm not clear on preferred docstring style. If someone lets me know what you'd like to see I'll happily add docstrings and doctests to cover this refactor!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-777082066
> > > I'm surprised you're not seeing similar performance improvements; would you mind sharing how you've tested it?
> >
> >
> > I just updated the notebook I shared earlier with your latest code.
>
> It looks like you're still calling `convert3`, not `convert4` in those benchmarks!
😱 Can't believe I made such a stupid mistake.
The updated results look good now. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773874994
hi @ktmud, thanks for picking this up!
I think I missed something in my original profiling; I've just checked again on my side and I'm seeing the same results you are, across small and large dataframes and varying fractions of large integers.
I'll admit I'm used to the 'optimisation' I proposed generally being orders of magnitude faster than manual iteration over each value within a dataframe so I was guilty of confirmation bias when I reviewed my original, mistaken benchmarking, but the context of converting the df to a list of dicts and keeping it in that format is the dominant factor here and as such I'm happy to close this out at this stage (unless anyone can think of a better way to handle this - maybe a more performant alternative to pandas `DataFrame.to_dict(orient="records")`?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (d2ec8c5) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `0.06%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 66.94% -0.07%
==========================================
Files 1022 489 -533
Lines 50102 28717 -21385
Branches 5191 0 -5191
==========================================
- Hits 33574 19225 -14349
+ Misses 16397 9492 -6905
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.94% <85.71%> (+2.86%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <85.71%> (-8.34%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| ... and [572 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...d2ec8c5](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.28%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.29% +0.28%
==========================================
Files 1022 489 -533
Lines 50102 28688 -21414
Branches 5191 0 -5191
==========================================
- Hits 33574 19306 -14268
+ Misses 16397 9382 -7015
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.29% <83.33%> (+3.21%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/examples/utils.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdXRpbHMucHk=) | `32.00% <0.00%> (-1.34%)` | :arrow_down: |
| ... and [567 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud merged pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #12806:
URL: https://github.com/apache/superset/pull/12806
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774447881
I've added the check/warning and updated my [benchmarking notebook](https://colab.research.google.com/drive/1_b2KKFrscNsbzceocxNpoNHGTS-IHbSL?usp=sharing) to include the latest code - I'm still seeing a 2x improvement for large dataframes, and more like 5-6x speedup on smaller dataframes. Either way, I think this is good to go now!
I'm surprised you didn't see similar improvements in your tests - could you share what you've tried so I can have a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773988790
I've had another look at this, and dropping pandas `DataFrame.to_dict()` in favour of the underlying operation means we can do the integer conversion during the same single loop over all records in the dataframe, rather than looping over the whole dataframe twice to get to where we need to end up.
At the risk of making myself look stupid, this seems to result in a 2x speedup on my local tests this time round, but I'd appreciate a second pair of eyes on this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (da18180) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 66.96% -0.05%
==========================================
Files 1022 489 -533
Lines 50102 28712 -21390
Branches 5191 0 -5191
==========================================
- Hits 33574 19228 -14346
+ Misses 16397 9484 -6913
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.96% <100.00%> (+2.88%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| ... and [571 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...ba92912](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774161823
@ktmud I've pushed some further optimisations and added a benchmarking notebook (see edited PR description). Looks like a ~2x speedup across a range of dataframe sizes now 👍
One thing to watch out for with this approach - we lose the pandas [sense-check](https://github.com/pandas-dev/pandas/blob/c5a13488414043397faf9f37ac3f2323caa15e6b/pandas/core/frame.py#L1568) that all columns have unique names and hence the warning about the resulting dicts not reflecting the source data if they aren't.
It'd be easy enough to add this check into our function here too - either via warnings.warn() or through the logger. What would be preferred?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (e426840) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.19%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.20% +0.19%
==========================================
Files 1022 489 -533
Lines 50102 28685 -21417
Branches 5191 0 -5191
==========================================
- Hits 33574 19278 -14296
+ Misses 16397 9407 -6990
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.20% <100.00%> (+3.12%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [572 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...e426840](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl removed a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl removed a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774447881
I've added the check/warning and updated my [benchmarking notebook](https://colab.research.google.com/drive/1_b2KKFrscNsbzceocxNpoNHGTS-IHbSL?usp=sharing) to include the latest code - I'm still seeing a 2x improvement for large dataframes, and more like 5-6x speedup on smaller dataframes. Either way, I think this is good to go now!
I'm surprised you didn't see similar improvements in your tests - could you share what you've tried so I can have a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] junlincc commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-777086228
@TColl Thank you so much for your contribution!
for documentation purpose, @ktmud could you comment here what/ or if there is any risks associate with this refractor? thanks!
cc @guhan
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774748650
> I'm surprised you're not seeing similar performance improvements; would you mind sharing how you've tested it?
I just updated the notebook I shared earlier with your code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl removed a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl removed a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774019433
One thing to watch out for with this approach - we lose the [pandas sense-check](https://github.com/pandas-dev/pandas/blob/cb1486c8ef53db474248a0f9b81937b487388431/pandas/core/frame.py#L1568) that all columns have unique names and hence the warning about the resulting dicts not reflecting the source data if they aren't.
It'd be easy enough to add this check into our function here too - either via `warnings.warn()` or through the logger. What would be preferred?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-769824500
For what it's worth, this could be sped up further (at the expense of readability) - Would you prefer to use pandas builtins or is a dict comprehension OK here?
```
def _convert_big_integers(dframe: pd.DataFrame) -> pd.DataFrame:
return pd.DataFrame(
{
col: dframe[col].map(
lambda v: str(v)
if isinstance(v, int) and abs(v) > JS_MAX_INTEGER
else v
)
for col in dframe.columns
}
)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773874994
hi @ktmud, thanks for picking this up!
I think I missed something in my original profiling; I've just checked again on my side and I'm seeing the same results you are, across small and large dataframes and varying fractions of large integers.
I'll admit I'm used to the 'optimisation' I proposed generally being orders of magnitude faster than manual iteration over each value within a dataframe so I was guilty of confirmation bias when I reviewed my original, mistaken benchmarking, but the context of converting the df to a list of dicts and keeping it in that format is the dominant factor here and as such I'm happy to close this out at this stage (unless anyone can think of a better way to handle this - maybe a more performant alternative to pandas `DataFrame.to_dict(orient="records")`?)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774161823
@ktmud I've pushed some further optimisations and added a benchmarking notebook (see edited PR description). Looks like a ~2x speedup on a range of dataframe sizes now 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774237939
> Either way, it'd be easy enough to add this check into our function here too - via warnings.warn() or through the logger. What would be preferred?
Let's use `warnings.warn()` just to be consistent with current behavior.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774748650
> I'm surprised you're not seeing similar performance improvements; would you mind sharing how you've tested it?
I just updated the notebook I shared earlier with your latest code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (32a8c75) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `16.52%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
===========================================
- Coverage 67.01% 50.48% -16.53%
===========================================
Files 1022 476 -546
Lines 50102 17154 -32948
Branches 5191 4435 -756
===========================================
- Hits 33574 8660 -24914
+ Misses 16397 8494 -7903
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.48% <ø> (-0.44%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/welcome/Welcome.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9XZWxjb21lLnRzeA==) | `2.94% <0.00%> (-85.95%)` | :arrow_down: |
| ... and [882 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...32a8c75](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-777101742
@junlincc It's a simple performance optimization. Risks are captured by passing the existing unit tests, as described in Test Plan.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774983885
> > I'm surprised you're not seeing similar performance improvements; would you mind sharing how you've tested it?
>
> I just updated the notebook I shared earlier with your latest code.
It looks like you're still calling `convert3`, not `convert4` in those benchmarks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-773885788
Let's keep this PR open for a couple of more days and see if people have ideas. The original code had a `TODO: refactor this`, so I'd assume the original author didn't like manual loops, either.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-770099433
I'm a huge fan of performance optimization PRs so I did some benchmarking and added one of my own optimizations:
https://colab.research.google.com/drive/1M6wrvyIBIs079IC8RUGCT5I5zOxwOZ33#scrollTo=Fcie22gnZHqr
Based on my tests, the original method is actually the fastest (the result is consistent on both my local machine and Google Collab). Could you share how you tested 3x performance gain with `Series.map`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (e426840) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **increase** coverage by `0.31%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
+ Coverage 67.01% 67.32% +0.31%
==========================================
Files 1022 489 -533
Lines 50102 28713 -21389
Branches 5191 0 -5191
==========================================
- Hits 33574 19331 -14243
+ Misses 16397 9382 -7015
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.32% <100.00%> (+3.24%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `89.65% <0.00%> (-10.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/examples/utils.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdXRpbHMucHk=) | `32.00% <0.00%> (-1.34%)` | :arrow_down: |
| ... and [561 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...e426840](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (d2ec8c5) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `0.06%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 66.94% -0.07%
==========================================
Files 1022 489 -533
Lines 50102 28715 -21387
Branches 5191 0 -5191
==========================================
- Hits 33574 19223 -14351
+ Misses 16397 9492 -6905
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `66.94% <85.71%> (+2.86%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <85.71%> (-8.34%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-13.80%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| ... and [573 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...d2ec8c5](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov-io edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-768998745
# [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=h1) Report
> Merging [#12806](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=desc) (9a795d1) into [master](https://codecov.io/gh/apache/superset/commit/32f2c45f93693156f1cf88c2c5223684c31d3f20?el=desc) (32f2c45) will **decrease** coverage by `4.92%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12806 +/- ##
==========================================
- Coverage 67.01% 62.09% -4.93%
==========================================
Files 1022 969 -53
Lines 50102 46031 -4071
Branches 5191 4485 -706
==========================================
- Hits 33574 28581 -4993
- Misses 16397 17450 +1053
+ Partials 131 0 -131
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `52.97% <ø> (+2.04%)` | :arrow_up: |
| javascript | `?` | |
| python | `67.58% <83.33%> (+3.50%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `90.90% <83.33%> (-9.10%)` | :arrow_down: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/nativeFilters/ScopingTree.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvU2NvcGluZ1RyZWUudHN4) | `6.25% <0.00%> (-93.75%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [443 more](https://codecov.io/gh/apache/superset/pull/12806/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=footer). Last update [32f2c45...9a795d1](https://codecov.io/gh/apache/superset/pull/12806?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl edited a comment on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
TColl edited a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774161823
@ktmud I've pushed some further optimisations and added a benchmarking notebook (see edited PR description). Looks like a ~2x speedup across a range of dataframe sizes now 👍
One thing to watch out for with this approach - we lose the pandas [sense-check](https://github.com/pandas-dev/pandas/blob/c5a13488414043397faf9f37ac3f2323caa15e6b/pandas/core/frame.py#L1568) that all columns have unique names and hence the warning about the resulting dicts not reflecting the source data if they aren't. I guess the issue of duplicate column names is more of a problem for individual file uploads, rather than anything we pull from an SQL DB, so this might not be a problem at all.
Either way, it'd be easy enough to add this check into our function here too - via warnings.warn() or through the logger. What would be preferred?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl commented on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774019433
One thing to watch out for with this approach - we lose the [pandas sense-check](https://github.com/pandas-dev/pandas/blob/cb1486c8ef53db474248a0f9b81937b487388431/pandas/core/frame.py#L1568) that all columns have unique names and hence the warning about the resulting dicts not reflecting the source data if they aren't.
It'd be easy enough to add this check into our function here too - either via `warnings.warn()` or through the logger. What would be preferred?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #12806: refactor: speed up conversion from dataframe to list of records
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-774750048
@TColl I think this looks good now. Congrats on landing your first Superset PR! 🥳
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] TColl removed a comment on pull request #12806: refactor: vectorise big integer to string type conversion
Posted by GitBox <gi...@apache.org>.
TColl removed a comment on pull request #12806:
URL: https://github.com/apache/superset/pull/12806#issuecomment-769824500
For what it's worth, this could be sped up further (at the expense of readability) - Would you prefer to use pandas builtins or is a dict comprehension OK here?
```
def _convert_big_integers(dframe: pd.DataFrame) -> pd.DataFrame:
return pd.DataFrame(
{
col: dframe[col].map(
lambda v: str(v)
if isinstance(v, int) and abs(v) > JS_MAX_INTEGER
else v
)
for col in dframe.columns
}
)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org