msyavuz opened a new pull request, #43384:
URL: https://github.com/apache/superset/pull/43384

   ### SUMMARY
   
   `renameOperator` rewrites `SUM(sales)__1 week ago` to `1 week ago` and drops 
the `__` separator. The backend `contribution` operator groups columns by that 
suffix (`get_column_groups` in 
`superset/utils/pandas_postprocessing/contribution.py`) so each time shift 
normalizes on its own. Timeseries `buildQuery` ran rename first, so the current 
period and the shifted period landed in the same bucket and a row contribution 
split 100% between them instead of per period.
   
   This swaps the two operators. Timeseries is the only plugin using 
`contributionOperator`, and post-processing gets rebuilt from form data on 
every query, so saved charts pick this up without a migration. Note the 
`time_shifts` grouping added in #28368 never matched for any comparison type, 
since rename always strips the suffix.
   
   Same pandas input, row contribution with a `1 week ago` shift:
   
   ```
   before:  current [0.125, 0.375]   shifted [0.25, 0.25]   # the whole row 
sums to 1
   after:   current [0.25,  0.75 ]   shifted [0.5,  0.5 ]   # each period sums 
to 1
   ```
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Skipped. The chart layout is identical, only the series values change, and 
the numbers above show that.
   
   ### TESTING INSTRUCTIONS
   
   1. Line chart on `Vehicle Sales`, metric `SUM(sales)`, dimension 
`product_line`.
   2. Contribution Mode `Row`, Stacked Style `Stack`.
   3. Filter `order_date` to Jan 2003 through Dec 2003.
   4. Time shift `1 week ago`, comparison type `Actual values`.
   5. Run the query. The current-period series stack to 100% and the shifted 
series stack to 100% separately.
   
   Unit test: `npm run test -- 
plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts`
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue: No
   - [ ] Required feature flags: None
   - [x] Changes UI: Yes, contribution values in stacked ECharts timeseries 
charts that use a time shift
   - [ ] Includes DB Migration: No
   - [ ] Introduces new feature or API: No
   - [ ] Removes existing feature or API: No
   
   https://claude.ai/code/session_01UUEazXwEndwJBFkXJrxRwA
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to