Skip to content

support NaN values in numeric grouping variables in dummyvar. - #481

Open
AvanishSalunke wants to merge 1 commit into
gnu-octave:mainfrom
AvanishSalunke:dummyvar
Open

AvanishSalunke wants to merge 1 commit into
gnu-octave:mainfrom
AvanishSalunke:dummyvar

Conversation

@AvanishSalunke

Copy link
Copy Markdown
Contributor

dummyvar treats NaN values and undefined categorical levels in group as missing data and returns NaN values in D.

a NaN in group would just throw a error instead of producing a NaN row. Fixed it by letting NaN through validation and filling NaN across that grouping variable's own block of dummy columns for affected rows.

BEFORE:

octave:2> g = [1; 2; NaN; 3; 2];
octave:3> D = dummyvar(g)
error: dummyvar: numeric grouping variable must explicitly contain positive integers.
error: called from
    dummyvar at line 98 column 7

AFTER:

octave:2> g = [1; 2; NaN; 3; 2];
octave:3> D = dummyvar(g)
D =

     1     0     0
     0     1     0
   NaN   NaN   NaN
     0     0     1
     0     1     0

MATLAB:

>> g = [1; 2; NaN; 3; 2];

>> D = dummyvar(g)

D =

     1     0     0
     0     1     0
   NaN   NaN   NaN
     0     0     1
     0     1     0

@pr0m1th3as

Copy link
Copy Markdown
Member

The inconsistency you are closing is real. The categorical branch has filled NaN rows for <undefined> since it went in and the help text promises it, while the numeric branch raised, so the two halves of one documented rule disagreed. The cell array branch picks your change up through the recursion, which is the right outcome.

I checked the behaviour against MATLAB R2024a.

The per variable rule is correct. Your test on [1, 1; 2, NaN; 1, 2; 2, 1] asserts that a NaN fills only its own variable's block of columns and leaves the other variable's columns alone. MATLAB gives the same array:

     1     0     1     0
     0     1   NaN   NaN
     1     0     0     1
     0     1     1     0

The cell array and the row vector agree as well, so that test and the sentence you added to the help text both stand.

A grouping variable that is entirely NaN needs handling. max with 'omitnan' returns NaN for such a column, so sum (K) is NaN and zeros raises:

>> dummyvar ([NaN; NaN])
error: conversion of nan to int64_t value failed
>> dummyvar ([1, NaN; 2, NaN])
error: conversion of nan to int64_t value failed
>> dummyvar ([1, NaN, 2; 2, NaN, 1])
error: conversion of nan to int64_t value failed

On main all three raise this function's own error naming the rule they break, so as the patch stands a clear message is replaced by an internal one. Your premise is that a NaN is missing data rather than an error, and a variable that is entirely missing is the limit of that premise rather than a separate kind of nonsense.

MATLAB does not raise there. The variable contributes no columns and the caller is warned, so [NaN; NaN] returns a 2 by 0 and [1, NaN, 2; 2, NaN, 1] returns the four columns the other two variables earn. This does the same, and I have checked that it reproduces every case above and keeps all 21 of your tests passing:

    K = max (g, [], 1, 'omitnan');
    allnan = find (isnan (K));
    if (! isempty (allnan))
      warning (strcat ("dummyvar: the following grouping variables contain", ...
                       " only NaN values and produce no dummy variables:", ...
                       " %s."), mat2str (allnan));
      K(allnan) = 0;
    endif
    D = zeros (nr, sum (K));

Please add a test for it. Save and restore the warning state the way the blocks in crosstab do, rather than switching warnings on.

Three smaller points:

  • The opening paragraph of the help text still says D is a matrix containing ones and zeros. It can hold NaN now.
  • Your NaN note is on the matrix bullet only. The vector bullet above it is unchanged, and your own first test uses a vector.
  • "omitnan" should be 'omitnan'. Single quotes are the default for option strings; double quotes are reserved for the strings passed to error, warning and the printf family.

One last thing, which is not yours and needs no change here. Inf gets past the validation, being positive and its own fix, so dummyvar ([1; Inf]) reaches zeros (nr, Inf) and fails with an error about Octave's index type that names neither Inf nor this function. MATLAB rejects it outright. Adding || any (isinf (gv)) to the test you are already editing would close it, or it can go in a separate pull request.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants