You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org> on 2026/05/16 09:39:01 UTC
[PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Cloud-Architect-Emma opened a new pull request, #40190:
URL: https://github.com/apache/superset/pull/40190
Fixes #39951
## Problem
ClickHouse (and some other backends) return numeric SUM() columns as
strings. The existing `infer_objects()` call does not convert these to
numeric types, causing `pandas/core/nanops.py` to raise:
`TypeError: Could not convert string '655' to numeric`
## Fix
Replace `infer_objects()` with `pd.to_numeric(errors="ignore")` which
correctly converts string-encoded numerics while leaving truly
non-numeric columns unchanged.
## Testing
The existing `df_metrics_to_num` tests cover this path.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3294155288
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
Review Comment:
This import is unused and should be removed as `pre-commit` has alerted
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4484001500
Hi @hainenber, updated the fix to be safer, now it only applies
pd.to_numeric when all non-null values in the column are successfully
convert to numeric (i.e. the null pattern is preserved). This way
it only affects columns where the backend returned numeric values
as strings (like ClickHouse SUM()), and leaves truly mixed/non-numeric
columns unchanged.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466904606
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466494743
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 56b724d151cb699ce4067e645d7d82092b362768 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a083b354262be000858068f |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40190--superset-docs-preview.netlify.app](https://deploy-preview-40190--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpBDkAwEADAv-yZ2ChN4weeodsVDbpNqRDxd73NYV6ISfZ4wgBQgU1ToKV49ndDm6d1kXxwHfLOyVNNEi5Oh5dQsuO4yTO60vWERlnVd61uLSOi6Q1qM8P3Aw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40190--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466501095
<!-- Bito Reply -->
The flagged issue is valid. Using `errors='ignore'` in `pd.to_numeric` leaves the column as an object type if any value is non-numeric, which can cause downstream numeric operations to fail. The suggested fix is to use `errors='coerce'` instead, which converts invalid entries to `NaN` and ensures the column becomes numeric. Here's the fix for the specific line in `dataframe_utils.py`:
**superset/common/utils/dataframe_utils.py**
```
df[col] = pd.to_numeric(df[col], errors='coerce')
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466486838
Hi @konmitya, I've opened a PR to fix this: apache/superset#40190
The root cause is that ClickHouse returns numeric SUM() columns as
strings, and the existing `infer_objects()` call doesn't convert them.
The fix replaces it with `pd.to_numeric(errors="ignore")` which
correctly handles string-encoded numerics.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4467803249
Hi @hainenber, all checks are passing and the fix has been updated
to use errors="coerce" as suggested by the code review bots.
Could you take a look when you get a chance?
Thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4529816801
Hi @hainenber, the only failing check is `Python-Integration / test-postgres (next)`
which is failing due to a pre-existing flaky test in
`tests/integration_tests/queries/saved_queries/api_tests.py::TestSavedQueryApi::test_get_saved_query`
with `AssertionError: assert 'a second ago' == 'now'`, a timing race condition
completely unrelated to this PR.
All other required checks are passing. Could you re-review and approve?
Thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3252848286
##########
superset/common/utils/dataframe_utils.py:
##########
@@ -57,7 +57,7 @@ def df_metrics_to_num(df: pd.DataFrame, query_object: QueryObject) -> None:
if dtype.type == np.object_ and col in query_object.metric_names:
# soft-convert a metric column to numeric
# will stay as strings if conversion fails
- df[col] = df[col].infer_objects()
+ df[col] = pd.to_numeric(df[col], errors="ignore")
Review Comment:
Good catch! Updated to use errors="coerce" so valid numeric strings
are converted and invalid values become NaN rather than leaving the
entire column as object type.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4663371104
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4507321915
Hi @hainenber, I've updated the fix based on your feedback. The implementation now only converts a column when ALL non-null values successfully convert to numeric, making it safe for mixed-type columns from any backend:
```python
converted = pd.to_numeric(df[col], errors="coerce")
if converted.notna().eq(df[col].notna()).all():
df[col] = converted
```
This is safer than the original `infer_objects()` and scoped by behavior rather than by database backend, it only affects columns where the backend returned genuinely numeric values as strings (like ClickHouse SUM()). Could you re-review? Thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4497420384
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4526117818
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466495422
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40190?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 54.98%. Comparing base ([`d62f154`](https://app.codecov.io/gh/apache/superset/commit/d62f1546aa450bc74821a229edf0ad562d13d6a9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`56b724d`](https://app.codecov.io/gh/apache/superset/commit/56b724d151cb699ce4067e645d7d82092b362768?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 337 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40190?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/common/utils/dataframe\_utils.py](https://app.codecov.io/gh/apache/superset/pull/40190?src=pr&el=tree&filepath=superset%2Fcommon%2Futils%2Fdataframe_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40190?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40190 +/- ##
==========================================
- Coverage 64.58% 54.98% -9.61%
==========================================
Files 2564 2585 +21
Lines 133576 136769 +3193
Branches 31033 31483 +450
==========================================
- Hits 86271 75199 -11072
- Misses 45813 60907 +15094
+ Partials 1492 663 -829
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.45% <0.00%> (-0.43%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.14% <0.00%> (-0.52%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.21% <0.00%> (-20.89%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40190/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40190?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3294665885
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
Review Comment:
Hi @hainenber, I've removed the unused import numpy as np from the test file as flagged by pre-commit. Could you re-review when you get a chance? Thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4608593340
@hainenber @bgreenlee @frafra @ryw, could you please review at your convenience?
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4466483809
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4537545148
@hainenber, please can you review at you convience.
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4649015755
Hi maintenance, can PR be reviewed?
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #40190:
URL: https://github.com/apache/superset/pull/40190
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3252625012
##########
superset/common/utils/dataframe_utils.py:
##########
@@ -57,7 +57,7 @@ def df_metrics_to_num(df: pd.DataFrame, query_object: QueryObject) -> None:
if dtype.type == np.object_ and col in query_object.metric_names:
# soft-convert a metric column to numeric
# will stay as strings if conversion fails
- df[col] = df[col].infer_objects()
+ df[col] = pd.to_numeric(df[col], errors="ignore")
Review Comment:
**Suggestion:** Using `errors="ignore"` can leave the entire metric column as object/string when even one value is non-numeric, so downstream numeric aggregations can still fail with the same `Could not convert string ... to numeric` error. Use coercive conversion for metric columns (eg converting invalid entries to `NaN`) so valid numeric strings are still parsed and the column becomes numeric. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Chart data requests using QueryContextProcessor may crash.
- ❌ Rolling/aggregate post-processing fails on ClickHouse metric strings.
- ⚠️ Contribution/other numeric features skip or mis-handle metrics.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. A chart request is executed through the chart data pipeline:
`QueryContextProcessor.get_payload()` at
`superset/common/query_context_processor.py:88-126` calls `get_query_results(...)`, which
in turn invokes `_get_full()` at `superset/common/query_actions.py:153-172`, obtaining a
`QueryContext` and calling `query_context.get_df_payload(query_obj, ...)` to retrieve a
pandas DataFrame.
2. `QueryContext.get_df_payload()` delegates to `QueryContextProcessor.get_df_payload()`
at `superset/common/query_context_processor.py:80-60` (around lines 82-60 in that file),
which calls `self.get_query_result(query_obj)`; this delegates to the datasource
implementation `get_query_result()` in `superset/models/helpers.py:107-120`, which first
normalizes the DataFrame via `normalize_df(df, query_object)` at
`superset/models/helpers.py:32-105`.
3. Inside `normalize_df`, after datetime normalization, metrics are normalized by calling
`dataframe_utils.df_metrics_to_num(df, query_object)` at
`superset/models/helpers.py:99-102`, which executes `df_metrics_to_num` defined in
`superset/common/utils/dataframe_utils.py:15-21`. This function iterates over columns and,
for metric columns with `dtype.type == np.object_`, runs `df[col] = pd.to_numeric(df[col],
errors="ignore")` (line 60 in the PR). When the ClickHouse backend returns a metric column
(e.g. `sum__metric`) as strings with at least one non-numeric value (for example `"655"`
and `"total"`), `pd.to_numeric(..., errors="ignore")` leaves the entire Series unchanged
as `object`, so the metric column remains a string-typed column instead of becoming
numeric.
4. The resulting `df` with metric columns still of dtype `object` is then used by
downstream numeric code paths. For instance, post-processing operations such as rolling
windows or aggregations use numpy/pandas numeric functions, e.g. the rolling operator in
`superset/utils/pandas_postprocessing/rolling.py:31-88` builds `df_rolling = df.loc[:,
columns.keys()]` and then calls `df_rolling = getattr(df_rolling,
rolling_type)(**rolling_type_options)`. When `rolling_type` is a numeric operation like
`"sum"` and the metric column is still an `object` Series with string values, pandas'
internal `nanops` machinery (in `pandas/core/nanops.py`) attempts to convert those strings
to numeric and raises `TypeError: Could not convert string '655' to numeric`, causing the
chart request (triggered via `_get_full` / `get_query_results`) to fail. This behavior
contrasts with the legacy `BaseViz.df_metrics_to_num` implementation in
`superset/viz.py:16-23`, which uses `pd.to_numeric(..., errors="coerce")`, coercing
invalid metric entries to `NaN` while ensuring the overall column becomes numeric.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fcommon%2Futils%2Fdataframe_utils.py%0A%2A%2ALine%3A%2A%2A%2060%3A60%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Using%20%60errors%3D%22ignore%22%60%20can%20leave%20the%20entire%20metric%20column%20as%20object%2Fstring%20when%20even%20one%20value%20is%20non-numeric%2C%20so%20downstream%20numeric%20aggregations%20can%20still%20fail%20with%20the%20same%20%60Could%20not%20convert%20string%20...%20to%20numeric%60%20error.%20Use%20coercive%20conversion%20for%20metric%20columns%20%28eg%20converting%20invalid%20entries%20to%20%60NaN%60%29%20so%20valid%20numeric%20strings%20are%20still%20parsed%20and%20the%20column%20becomes%20numeric.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%
20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fcommon%2Futils%2Fdataframe_utils.py%0A%2A%2ALine%3A%2A%2A%2060%3A60%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Using%20%60errors%3D%22ignore%22%60%20can%20leave%20the%20entire%20metric%20column%20as%20object%2Fstring%20when%20even%20one%20value%20is%20non-numeric%2C%20so%20downstream%20numeric%20aggregations%20can%20still%20fail%20with%20the%20same%20%60Could%20not%20convert%20string%20...%20to%20numeric%60%20error.%20Use%20coercive%20conversion%
20for%20metric%20columns%20%28eg%20converting%20invalid%20entries%20to%20%60NaN%60%29%20so%20valid%20numeric%20strings%20are%20still%20parsed%20and%20the%20column%20becomes%20numeric.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/common/utils/dataframe_utils.py
**Line:** 60:60
**Comment:**
*Logic Error: Using `errors="ignore"` can leave the entire metric column as object/string when even one value is non-numeric, so downstream numeric aggregations can still fail with the same `Could not convert string ... to numeric` error. Use coercive conversion for metric columns (eg converting invalid entries to `NaN`) so valid numeric strings are still parsed and the column becomes numeric.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40190&comment_hash=0dbafd11a269f136a721e1ef4f04c36d483b600bfabdf462252c2152f105cddc&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40190&comment_hash=0dbafd11a269f136a721e1ef4f04c36d483b600bfabdf462252c2152f105cddc&reaction=dislike'>👎</a>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3297104872
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
@@ -48,3 +49,27 @@ def test_is_datetime_series():
datetime.datetime(2018, 1, 1), datetime.datetime(2018, 2, 1)
).to_series()
)
+
+
+def test_df_metrics_to_num_converts_string_numerics():
+ """Test that string-encoded numeric columns (e.g. from ClickHouse) are converted."""
+ query_object = MagicMock()
+ query_object.metric_names = ["sum_col", "mixed_col", "text_col"]
+ df = pd.DataFrame(
+ {
+ "sum_col": pd.array(["100", "200", "300"], dtype=object),
+ "mixed_col": pd.array(["1", "not_a_number", "3"], dtype=object),
+ "text_col": pd.array(["foo", "bar", "baz"], dtype=object),
+ "dim_col": pd.array(["a", "b", "c"], dtype=object),
Review Comment:
add tests\unit_tests\common\test_dataframe_utils.py
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "Cloud-Architect-Emma (via GitHub)" <gi...@apache.org>.
Cloud-Architect-Emma commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4525034201
Hi @hainenber, added a unit test to `tests/unit_tests/common/test_dataframe_utils.py`
that covers:
- String-encoded numeric columns (e.g. ClickHouse SUM()) → converted to numeric
- Mixed columns with non-numeric values → NOT converted
- Non-numeric text columns → NOT converted
- Non-metric columns → NOT touched
Could you re-review? Thank you!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3295300259
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
@@ -48,3 +49,27 @@ def test_is_datetime_series():
datetime.datetime(2018, 1, 1), datetime.datetime(2018, 2, 1)
).to_series()
)
+
+
+def test_df_metrics_to_num_converts_string_numerics():
+ """Test that string-encoded numeric columns (e.g. from ClickHouse) are converted."""
+ query_object = MagicMock()
+ query_object.metric_names = ["sum_col", "mixed_col", "text_col"]
+ df = pd.DataFrame(
+ {
+ "sum_col": pd.array(["100", "200", "300"], dtype=object),
+ "mixed_col": pd.array(["1", "not_a_number", "3"], dtype=object),
+ "text_col": pd.array(["foo", "bar", "baz"], dtype=object),
+ "dim_col": pd.array(["a", "b", "c"], dtype=object),
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect test data construction</b></div>
<div id="fix">
Columns created with `pd.array(..., dtype=object)` become `str` dtype in DataFrames, not `np.object_`. The implementation at line 57 checks `dtype.type == np.object_`, which will never match these columns. Test data must use `pd.Series(..., dtype=object)` instead.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- tests/unit_tests/common/test_dataframe_utils.py
+++ tests/unit_tests/common/test_dataframe_utils.py
@@ -57,10 +57,10 @@ def test_df_metrics_to_num_converts_string_numerics():
query_object.metric_names = ["sum_col", "mixed_col", "text_col"]
df = pd.DataFrame(
{
- "sum_col": pd.array(["100", "200", "300"], dtype=object),
- "mixed_col": pd.array(["1", "not_a_number", "3"], dtype=object),
- "text_col": pd.array(["foo", "bar", "baz"], dtype=object),
- "dim_col": pd.array(["a", "b", "c"], dtype=object),
+ "sum_col": pd.Series(["100", "200", "300"], dtype=object),
+ "mixed_col": pd.Series(["1", "not_a_number", "3"], dtype=object),
+ "text_col": pd.Series(["foo", "bar", "baz"], dtype=object),
+ "dim_col": pd.Series(["a", "b", "c"], dtype=object),
}
)
```
</div>
</details>
</div>
<small><i>Code Review Run #fc4dc1</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4529298946
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3297105282
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
@@ -48,3 +49,27 @@ def test_is_datetime_series():
datetime.datetime(2018, 1, 1), datetime.datetime(2018, 2, 1)
).to_series()
)
+
+
+def test_df_metrics_to_num_converts_string_numerics():
+ """Test that string-encoded numeric columns (e.g. from ClickHouse) are converted."""
+ query_object = MagicMock()
+ query_object.metric_names = ["sum_col", "mixed_col", "text_col"]
+ df = pd.DataFrame(
+ {
+ "sum_col": pd.array(["100", "200", "300"], dtype=object),
+ "mixed_col": pd.array(["1", "not_a_number", "3"], dtype=object),
+ "text_col": pd.array(["foo", "bar", "baz"], dtype=object),
+ "dim_col": pd.array(["a", "b", "c"], dtype=object),
Review Comment:
<!-- Bito Reply -->
I can only review and explain code in this pull request. I cannot modify, delete, commit, push, or merge files. To apply this change, please update the file in your branch and push the commit.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4529941494
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4534145275
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #40190:
URL: https://github.com/apache/superset/pull/40190#discussion_r3294155288
##########
tests/unit_tests/common/test_dataframe_utils.py:
##########
Review Comment:
This import is unused and should be removed as `pre-commit` has alerted
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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
Re: [PR] fix: use pd.to_numeric in df_metrics_to_num to handle string-encoded numerics from ClickHouse [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40190:
URL: https://github.com/apache/superset/pull/40190#issuecomment-4467419608
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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