semohr commented on PR #37396: URL: https://github.com/apache/superset/pull/37396#issuecomment-5711560341
> @semohr been meaning to loop back on this one. master has since grown its own path to column labels in the DataTable headers, dataset verbose_map prop-drilled through SliceHeaderControls / ResultsPaneOnDashboard / useResultsPane, plus the queriesResponse reuse so it skips a duplicate request. Curious whether that already covers what this PR set out to do, or if collabels is still filling a real gap I dont think drilling the verbose map down multiple layers and seperating data needed for one table between two requests is architectually a good idea (which is currently done). The current solution seems pretty hacky and looks like an AI thought of it to me 😅 I mean it works but architectually it is a strange choice. I think I commented with a bit more reasoning on the PR where this was introduced but ofc I only saw it after it was merged. > Digging through the conflicts, I noticed useGridResultTable on master also strips the __contribution suffix and parses JSON-encoded adhoc metric columns into readable labels client-side. extract_display_labels here doesn't do either, and the test covering it got deleted instead of ported over. If we do want the label logic to live server-side, that piece needs to come with it. Yeah, we should port this over if it is needed. I personally don't understand why this logic exist at all tho. Seems strange and pretty hacky that we encode data in the verbose map/labels in the first place. That should not be the concern of the labels imo. Wouldnt a boolean field for that somewhere (e.g. `is_contribution`) make way more sense? -- 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]
