Add a fill_nan method to dataframe and column

[rgommers]

Addresses half of gh-142 (fill_null is more complex, and not included here).

Note: this is reviewable now, but should be merged after gh-157 which introduces the null object).

[MarcoGorelli]

looks good (barring docs build error)

[rgommers]

It's green now. I had to do the null type annotation in string form to make Sphinx happy.

[MarcoGorelli]

looks good to me

[kkraus14]

A bit unrelated to this PR, but having null be typed differently feels like an anti-pattern here. It differentiates between a float scalar (which is implicitly nullable based on our current scalar definition) and a null scalar.

[rgommers]

float scalar (which is implicitly nullable based on our current scalar definition

We don't have numpy-style scalars (i.e., instances of a dtype) though? That's why we need a separate null object, so that one can construct a column containing nulls with column_from_sequence([1.5, 2.5, null, 4.5]).

We could add dtype instances and specify that null derives from float, but that seems like a huge can of worms for no gain at all. And I think the consensus from what we learned from numpy is that array scalars were a major design mistake.

[MarcoGorelli]

nitpick: we can construct a column with column_from_sequence([1.5, 2.5, null, 4.5], dtype='float64') (just pointing this out because it's early days, and I wouldn't want someone to see this and get confused)

[kkraus14]

And I think the consensus from what we learned from numpy is that array scalars were a major design mistake.

Yes, but I think the thing that was agreed as the correct path forward was 0d arrays which we don't have on the DataFrame side. Those 0d arrays are strongly typed and don't have to deal with nulls.

The issue that I see is that someone could do something like:

my_int_column = column_from_sequence([1, 2, None, 4], dtype='int32')
max_my_int_column = my_int_column.max(skip_null=False)  # Yields a ducktyped `null` scalar that is int32 typed. Is this `int` type or `null` type from a Typing perspective?

my_float_column = column_from_sequence([1.5, 2.5, max_my_int_column, 4.5], dtype='float64')  # Does this work if the max is `null`?

For example, PyArrow handles this by having an explicit NULL type (https://arrow.apache.org/docs/python/generated/pyarrow.null.html#pyarrow.null) and presumably has its underlying APIs and compute explicitly handle mixing NULL typed scalars / columns with other typed scalars / columns.

Maybe we just need an explicit NULL type and then 'null' here refers to a scalar of type NULL?

[rgommers]

Maybe we just need an explicit NULL type

We do have exactly that already: docs for null. The only reason the type annotation is 'null' rather than null is to avoid some circular import and Sphinx weirdness.

[kkraus14]

That is an object for a null scalar as opposed to a NULL data type. I.E. allowing a column to be typed NULL and extracting a null-valued scalar from that column has type NULL versus extracting a null-valued scalar from a float64 column has type float64.

It feels counter-intuitive that Columns are type-erased (i.e. just a Column class and no Int32Column, Float32Column, etc.) but the scalars that are contained within Columns are not.

[kkraus14]

Either way, this should go into a new issue instead of this PR. Just the typing felt a bit funky to me here.

I'll open a new issue for discussion and approve this.

[rgommers]

Thanks, a new issue sounds good for this. I had not thought before about a need for a null dtype; if there is one we should indeed consider it.

[jorisvandenbossche]

For example, PyArrow handles this by having an explicit NULL type

Small clarification here: while pyarrow indeed has a "null" data type, we also have type-specific null scalars for each data type. And so in your specific example, the max_my_int_column would actually be an int32 scalar (with the value of "null"), and not a scalar of the null data type.

[MarcoGorelli]

all good, thanks - @kkraus14 any further comments or good to go?

[rgommers]

This now has three approvals, so I'll get it in. Thanks all!