Choosing the default axis for fill_none

[agoose77]

Context

In #917 a new axis parameter is added to constrain the scope of fill_none to a particular layout depth. This should make the function more predictable. axis may be set to None, in which case fill_none will recurse to all axes.

However, as this new behaviour is breaking, it must be opted into explicitly, i.e. with ak.fill_none(array, axis=...). Once the deprecation period elapses, we will need to choose a default axis. For context, the current behaviour without an axis is to find the first option axis, starting from 0, and remove the option type.

New behaviour

For an example of the new behaviour, consider the following layout:

layout = (
    # current axis 0
    ak.layout.RegularArray(
        # current axis 1
        ak.layout.IndexedOptionArray64(
            ak.layout.Index64([0, 1, -1, 2, 3]),
            ak.layout.ListOffsetArray64(
                # current axis 2
                ak.layout.Index64([0, 3, 6, 9, 12]),
                ak.layout.IndexedOptionArray64(
                    ak.layout.Index64([0, 1, 2, 3, 4, 5, -1, 7, 8, 9, 10, 11]),
                    ak.layout.NumpyArray(np.arange(12)),
                ),
            ),
        ),
        2,
    )
)

This layout has the corresponding list representation:

[
    [
        [0, 1, 2],
        [3, 4, 5],
    ],
    [
        None,
        [None, 7, 8],
    ],
]

If we call fill_none with axis=0, then it will try to remove any Nones in the zeroth axis:

>>> ak.fill_none(layout, 1, axis=0).tolist()
... [
...     [
...         [0, 1, 2],
...         [3, 4, 5],
...     ],
...     [
...         None,
...         [None, 7, 8],
...     ],
... ]

This array does not have any Nones in the zeroth axis (because a bare RegularArray produces sublists of fixed size), and so it returns the same layout.

Now, if we call fill_none with axis=1, it will remove the Nones in the first axis:

>>> ak.fill_none(layout, 1, axis=1).tolist()
... [
...     [
...         [0, 1, 2],
...         [3, 4, 5],
...     ],
...     [
...         1,
...         [None, 7, 8],
...     ],
... ]

This does change the layout, because of the IndexedOptionArray64 that the RegularyArray builds the sublists from.

Finally, we have the axis=None behaviour. Clearly, all levels of None have been removed.

>>> ak.fill_none(layout, 1, axis=None).tolist()
... [
...     [
...         [0, 1, 2],
...         [3, 4, 5],
...     ],
...     [
...         1,
...         [1, 7, 8],
...     ],
... ]

Default axis?

So, the question remains, what should ak.fill_none(layout, <value>) do? There are some options:

1. Let axis=0 for symmetry with ak.is_none
2. Let axis=1 for symmetry with ak.pad_none (and some other existing functions)
3. Let axis=-1 for "the most likely" scenario that users are filling with scalar values

This Discussion is a welcome place for users to provide their opinions on what the new defaults should be. As @jpivarski says, now is the time to decide it whilst it's not depended upon!

[jpivarski]

I don't think axis=1 is a good choice. That's the default for ak.pad_none because that function rarely makes sense at axis=0, unlike ak.fill_none.

For me, I see the choice as being between axis=0 (because I feel like defaults should have minimal scope, with non-defaults expanding outward) and axis=None (because a literal reading of "fill with Nones" sounds like you're going to do it everywhere, on all levels).

@lgray, @nsmith-, and @raymondEhlers, you have all been involved in issues that mentioned ak.fill_none in the past. Do you have opinions on the default axis?

[agoose77]

Yes, the existing version of fill_none will introduce a number of Unions. I suppose this is really where careful usage of axis is always better than blindly calling fill_none with no axis.

[jpivarski]

If any choice of axis is less likely to lead to new UnionArrays, then that choice is better. UnionArrays cause troubles down the road.

I see now what you mean, @nsmith-, about axis=-1 being the least likely to lead to UnionArrays, because a user is most likely to pass a number as the fill value and maybe not notice that some lists are missing as well. I hadn't considered axis=-1, but maybe we should be considering this function as a thing like ak.values_astype (which only operates at axis=-1; ak.fill_none could default to that but have more general options).

The point about UnionArrays makes me more negative about axis=None. By definition, there isn't any fill value that would make multilple levels non-union.

[agoose77]

@jpivarski @nsmith this is something that I was just thinking about.

I don't think the argument against axis != -1 holds though; I do agree that filling on those axes will produce UnionArrays, but only if the user passes a scalar value. The issue is not which axis is being used, rather whether the user is using fill_axis propertly; fill_none with scalars is effectively a shorthand for union for outer axes. Despite this paragraph, I also agree that -1 is probably a better option because I suspect scalar fill values are more common (and therefore, axis = -1) 🙃

This now makes me strongly want to drop axis=None. Sometimes it's quite easy to lose sight of the problem that you're actually trying to solve. I don't think axis=None has any real value at all.

[raymondEhlers]

Just to add my experience: grepping around my repos, I see that I almost always use fill_none with pad_none to make arrays rectilinear, filling in some scalar value. In the other cases, I was trying to get rid of all option types (which is to say, I was expecting axis=None). But in practice, I think axis=-1 would have been enough to get what I wanted (plus, this unique case is only due to the option types that used to occur when writing parquet files).

Stepping back, this discussion made me realize that I wasn't really aware of the details of the current behavior until reading about them here. Naively, I had expected it to be axis=None. However, given the discussion above about unions and that axis=-1 would have practically fulfilled my use cases, axis=-1 seems reasonable to me

[jpivarski]

I agree: the main use is to make arrays rectilinear (and non-option-type). axis=None can't do that: you'd have to call ak.fill_none with different fill values for each level if you had missing values at multiple levels.

On the other hand, I don't think it's bad that axis=None was implemented. Union arrays are valid arrays, and sometimes someone would want one (and deal with the fact that not everything works for them). But it's clearer that it should not be the default.

Most users probably have missing values at only one level, which is why the weird behavior the function used to have worked. Using axis=-1 means you usually don't have to think about the type of the fill value: it would almost always be a number (some sort of leaf type). Having less to think about is a big plus.

All of this discussion has been great—I don't think either of us were considering axis=-1 before, but this is looking more like the right default. Without having this conversation, we very likely would have picked a bad default.

Does anyone here have objections to axis=-1?