Skip to content

Conversation

@SANDEEPNEGI07
Copy link

@SANDEEPNEGI07 SANDEEPNEGI07 commented Oct 25, 2025

…shmallow>=4.0.0

Fixes #33162 : update Prophet schema for Marshmallow>=4.0.0 compatibility

SUMMARY

Fixes Marshmallow 4.0 compatibility issue in ChartDataProphetOptionsSchema.

periods field was using min which was deprecated in the marshmallow>=4.0.0. This caused a TypeError: __init__() got an unexpected keyword argument 'min'

Used validate=Range() instead of min, which is compatible with both Marshmallow>=4.0.0.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend schema validation fix

Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've completed my review and didn't find any issues.

Files scanned
File Path Reviewed
superset/charts/schemas.py

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@dosubot dosubot bot added the change:backend Requires changing the backend label Oct 25, 2025
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Oct 25, 2025

Code Review Agent Run #b906ee

Actionable Suggestions - 0
Additional Suggestions - 2
  • superset/charts/schemas.py - 2
    • Line exceeds maximum length limit · Line 660-660
      Line 660 exceeds the 88-character limit (90 characters). Consider breaking the error message into multiple lines or using a shorter message.
      Code suggestion
       @@ -659,3 +659,4 @@
                # Use validate=Range() instead of min parameter for Marshmallow 4.0+ compatibility
                validate=[
                    Range(
                        min=0,
                        min_inclusive=True,
      -                error=_("periods` must be greater than or equal to 0"),
      +                error=_(
      +                    "`periods` must be greater than or equal to 0"
      +                ),
                    )
                ],
    • Missing trailing comma in list · Line 666-666
      Missing trailing comma after the `Range` validation object on line 666. Add a trailing comma to maintain consistency with Python style conventions.
      Code suggestion
       @@ -664,6 +664,6 @@
                        min_inclusive=True,
                        error=_("periods` must be greater than or equal to 0"),
      -            )
      +            ),
                ],
Review Details
  • Files reviewed - 1 · Commit Range: 5bdf534..5bdf534
    • superset/charts/schemas.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review
Copy link
Contributor

Interaction Diagram by Bito
sequenceDiagram
participant API as REST API Layer
participant QC as ChartDataQueryContextSchema
participant PO as ChartDataProphetOptionsSchema<br/>🔄 Updated | ●●○ Medium
participant Validator as Range Validator<br/>🟩 Added | ●●○ Medium
participant Prophet as prophet() Function
Note over PO: Marshmallow 4.0+ compatibility<br/>Replace min param with validate=Range()
API->>QC: POST /api/v1/chart/data
QC->>PO: Load post_processing options
PO->>Validator: Validate periods field
Validator-->>PO: periods >= 0 check
PO-->>Prophet: Pass validated options
Prophet-->>API: Return forecast results
Loading

Critical path: REST API Layer->ChartDataQueryContextSchema->ChartDataProphetOptionsSchema->Range Validator->prophet() Function

Note: The periods field in ChartDataProphetOptionsSchema is updated to use Marshmallow 4.0+ compatible Range validator instead of deprecated min parameter. This ensures compatibility with newer Marshmallow versions while maintaining the same validation logic (periods must be >= 0).

@SANDEEPNEGI07 SANDEEPNEGI07 changed the title Replace deprecated min parameter with validate=Range() to support mar… Fix(#33162): Replace deprecated min parameter with validate=Range() to support mar… Oct 25, 2025
@sadpandajoe sadpandajoe self-requested a review October 28, 2025 17:23
@sadpandajoe sadpandajoe changed the title Fix(#33162): Replace deprecated min parameter with validate=Range() to support mar… fix: Replace deprecated min parameter with validate=Range() to support marshmallow>=4.0.0 Oct 28, 2025
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.31%. Comparing base (76d897e) to head (5bdf534).
⚠️ Report is 2799 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #35844       +/-   ##
===========================================
+ Coverage   60.48%   71.31%   +10.83%     
===========================================
  Files        1931      600     -1331     
  Lines       76236    44009    -32227     
  Branches     8568     4767     -3801     
===========================================
- Hits        46114    31387    -14727     
+ Misses      28017    11375    -16642     
+ Partials     2105     1247      -858     
Flag Coverage Δ
hive 45.90% <ø> (-3.25%) ⬇️
javascript ?
postgres 70.44% <ø> (?)
presto 49.57% <ø> (-4.23%) ⬇️
python 71.28% <ø> (+7.77%) ⬆️
sqlite 70.04% <ø> (?)
unit 100.00% <ø> (+42.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Superset compatible with marshmallow>=4.0.0

1 participant