test broken#41775
Conversation
Code Review Agent Run #0f42a3Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
||
|
|
||
| def cast_to_num(value: float | int | str | None) -> float | int | None: | ||
| def cast_to_num(value: float | int | str) -> float | int | None: |
There was a problem hiding this comment.
Suggestion: This signature change drops None from the accepted input type, but the function’s documented/tested contract still includes None and the body calls value.isdigit() without a null guard. Passing None now will raise an exception instead of returning None, so restore None in the accepted type and keep an explicit None early return. [api mismatch]
Severity Level: Critical 🚨
- ❌ Integration test `test_cast_to_num` fails due to AttributeError.
- ⚠️ Numeric filter conversion in helpers.py uses cast_to_num.
- ⚠️ Function docs and doctests still include None input.Steps of Reproduction ✅
1. Run the integration test suite, specifically `test_cast_to_num` in
`tests/integration_tests/utils_tests.py:404-410`, which imports `cast_to_num` from
`superset.utils.core` at lines 42-46.
2. The test calls `cast_to_num(None)` (see `tests/integration_tests/utils_tests.py:409`),
expecting the function to return `None` as asserted in the test and documented by the
doctest examples in `superset/utils/core.py:56-59`.
3. The call reaches `cast_to_num` defined in `superset/utils/core.py:41-72`, where the
signature is `def cast_to_num(value: float | int | str) -> float | int | None:` and the
implementation executes:
- `if isinstance(value, (int, float)):` at line 65, which is false for `None`,
- then `if value.isdigit():` at line 67.
4. Because `value` is `None`, `value.isdigit()` raises `AttributeError: 'NoneType' object
has no attribute 'isdigit'`, causing the test and any real caller passing `None` (such as
numeric filter handling in `superset/models/helpers.py:2658-2673` which delegates to
`utils.cast_to_num(value)`) to fail unexpectedly instead of returning `None` as per the
documented contract.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/utils/core.py
**Line:** 420:420
**Comment:**
*Api Mismatch: This signature change drops `None` from the accepted input type, but the function’s documented/tested contract still includes `None` and the body calls `value.isdigit()` without a null guard. Passing `None` now will raise an exception instead of returning `None`, so restore `None` in the accepted type and keep an explicit `None` early return.
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|
The flagged issue is correct. The removal of superset/utils/core.py |
Code Review Agent Run #70397aActionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION