[GitHub] [systemds] BACtaki commented on pull request #1677: [SYSTEMDS-3413] Add support for row/col aggregation to countDistinct builtin function

[GitBox <gi...@apache.org>]

BACtaki commented on PR #1677:
URL: https://github.com/apache/systemds/pull/1677#issuecomment-1288337567

   > I like the PR, i do not see in the code why the tests would fail so i will force them to restart,
   The following careless edit was the culprit:
   ```
   @@ -2361,10 +2375,6 @@ public class DMLTranslator
    		case SUM:
    		case PROD:
    		case VAR:
   -		case COUNT_DISTINCT:
   -			currBuiltinOp = new AggUnaryOp(target.getName(), DataType.SCALAR, target.getValueType(),
   -					AggOp.valueOf(source.getOpCode().name()), Direction.RowCol, expr);
   -			break;
   ```
   
   > is the method supposed to be:
   > countDistinctCol(X) or countDistinct(X, "c")
   > Personally i would prefer if we had both, maybe @mboehm7 have an opinion?
   
   That is a great idea! I have added `countDistinctRow` and `countDistinctCol` aliases for `countDistinct(X, dir="r")` and `countDistinct(X, dir="c")` respectively.


-- 
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: dev-unsubscribe@systemds.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org