kz930 opened a new pull request, #7980: URL: https://github.com/apache/texera/pull/7980
### What changes were proposed in this PR? An operator property that takes several column names never had its `attributeTypeRules` checked. The `checkAttributeType` validator reads the property's value as a single attribute name and asks the compiling service for that name's type. A property holding a list handed it an array, no attribute matched, the type came back undefined, and the validator treated the property as not yet filled in and moved on. Single-column properties were unaffected, which is why the gap went unnoticed. Four operators on main declare a rule on such a property, so all four were inert: Scatter Matrix Chart's Selected Attributes, Parallel Coordinates Plot's dimensions, Radar Chart's valueColumns, and Radar Plot's selectedAttributes. The validator now reads both shapes as a list of names and checks each one on its own. The warning names the column that broke the rule rather than the property, which matters when a property holds several. Assembling that message inside the loop also removes the duplicated type lookup the outer catch block was doing, along with the note it left asking for exactly that. The `const` `$data` comparison still resolves a single name. Both rules that use it, Hash Join's and Interval Join's, name a single-column property. With the rule now reaching every named column, this also declares the one the sklearn advanced trainers were missing. Their Selected Features columns are passed to `fit` with no preprocessing, so a text or binary column fails inside scikit-learn with "could not convert string to float". Every other Texera attribute type is fitted, a timestamp included: it arrives as datetime64 and is read as epoch microseconds. Declaring this rule before the fix above would have had no effect. ### Any related issues, documentation, discussions? Closes #7979 ### How was this PR tested? Three cases were added to the existing `attributeTypeRules` suite in `operator-property-edit-frame.component.spec.ts`: a multi-column property whose columns all match, one where the second column violates the rule and the warning names that column, and one that names no columns at all. The suite passes with 222 tests. One case was added to `SklearnAdvancedBaseDescSpec`, reading the accepted set out of the generated operator schema. That suite passes with 8 tests. The accepted set itself was checked against scikit-learn rather than written from memory. Each Texera attribute type was fitted through `KNeighborsClassifier`, `KNeighborsRegressor`, `SVC` and `SVR`. Only string and binary raise. It was also checked by hand in the running application, on a workflow feeding one CSV source into both a Bar Chart and a Scatter Matrix Chart. Before the change, choosing a string column as the Bar Chart's Value Column warned and choosing the same column under the Scatter Matrix's Selected Attributes did not. After it, both warn. Selecting a numeric column and a string one together names the string one in the warning, not the property and not the column that was fine. One pre-existing behaviour is worth noting for a reviewer reproducing this: the warning appears once the field has been edited, not on reloading a workflow whose stored value already violates the rule. That is the same before and after, and the same for single-column properties. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) -- 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]
