Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functionality to apply Dtype metadata to ColumnBase #8373

Merged
merged 19 commits into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions python/cudf/cudf/core/column/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
as_column,
build_categorical_column,
build_column,
build_list_column,
build_struct_column,
column_empty,
column_empty_like,
column_empty_like_same_mask,
Expand All @@ -15,10 +17,10 @@
serialize_columns,
)
from cudf.core.column.datetime import DatetimeColumn # noqa: F401
from cudf.core.column.decimal import DecimalColumn # noqa: F401
from cudf.core.column.interval import IntervalColumn # noqa: F401
from cudf.core.column.lists import ListColumn # noqa: F401
from cudf.core.column.numerical import NumericalColumn # noqa: F401
from cudf.core.column.string import StringColumn # noqa: F401
from cudf.core.column.struct import StructColumn # noqa: F401
from cudf.core.column.timedelta import TimeDeltaColumn # noqa: F401
from cudf.core.column.interval import IntervalColumn # noqa: F401
from cudf.core.column.decimal import DecimalColumn # noqa: F401
36 changes: 16 additions & 20 deletions python/cudf/cudf/core/column/categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -1506,27 +1506,23 @@ def _concat(objs: MutableSequence[CategoricalColumn]) -> CategoricalColumn:
offset=codes_col.offset,
)

def _copy_type_metadata(
self: CategoricalColumn, other: ColumnBase
) -> ColumnBase:
"""Copies type metadata from self onto other, returning a new column.

In addition to the default behavior, if `other` is not a
CategoricalColumn, we assume other is a column of codes, and return a
CategoricalColumn composed of `other` and the categories of `self`.
"""
if not isinstance(other, cudf.core.column.CategoricalColumn):
other = column.build_categorical_column(
categories=self.categories,
codes=column.as_column(other.base_data, dtype=other.dtype),
mask=other.base_mask,
ordered=self.ordered,
size=other.size,
offset=other.offset,
null_count=other.null_count,
def _apply_type_metadata(
charlesbluca marked this conversation as resolved.
Show resolved Hide resolved
self: CategoricalColumn, dtype: Dtype
) -> CategoricalColumn:
if isinstance(dtype, CategoricalDtype):
self = column.build_categorical_column(
charlesbluca marked this conversation as resolved.
Show resolved Hide resolved
categories=dtype.categories._values,
codes=column.as_column(
self.codes.base_data, dtype=self.codes.dtype
),
mask=self.codes.base_mask,
ordered=dtype.ordered,
size=self.codes.size,
offset=self.codes.offset,
null_count=self.codes.null_count,
)
# Have to ignore typing here because it misdiagnoses super().
return super()._copy_type_metadata(other) # type: ignore

return self


def _create_empty_categorical_column(
Expand Down
147 changes: 97 additions & 50 deletions python/cudf/cudf/core/column/column.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from cudf.core.buffer import Buffer
from cudf.core.dtypes import (
CategoricalDtype,
Decimal64Dtype,
IntervalDtype,
ListDtype,
StructDtype,
Expand Down Expand Up @@ -1267,6 +1268,9 @@ def scatter_to_table(
}
)

def _apply_type_metadata(self: ColumnBase, dtype: Dtype) -> ColumnBase:
charlesbluca marked this conversation as resolved.
Show resolved Hide resolved
return self

def _copy_type_metadata(self: ColumnBase, other: ColumnBase) -> ColumnBase:
"""
Copies type metadata from self onto other, returning a new column.
Expand All @@ -1276,17 +1280,7 @@ def _copy_type_metadata(self: ColumnBase, other: ColumnBase) -> ColumnBase:
and the children of `other`.
* if none of the above, return `other` without any changes
"""
# TODO: This logic should probably be moved to a common nested column
# class.
if isinstance(other, type(self)):
if self.base_children and other.base_children:
base_children = tuple(
self.base_children[i]._copy_type_metadata(
other.base_children[i]
)
for i in range(len(self.base_children))
)
other.set_base_children(base_children)
other = other._apply_type_metadata(self.dtype)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method, along with _copy_type_metadata_from_arrow, is now a one-liner since all the logic is handled in apply. Is it worth it to keep the copy functions, or should we just replace all their appearances with the corresponding apply method call?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are really the same thing I'd vote to remove the old ones, less clutter, less confusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to avoid the indirection and just delete the methods.


return other

Expand Down Expand Up @@ -1603,6 +1597,84 @@ def build_interval_column(
)


def build_list_column(
indices: ColumnBase,
elements: ColumnBase,
mask: Buffer = None,
size: int = None,
offset: int = 0,
null_count: int = None,
) -> "cudf.core.column.ListColumn":
"""
Build a ListColumn

Parameters
----------
indices : ColumnBase
Column of list indices
elements : ColumnBase
Column of list elements
mask: Buffer
Null mask
size: int, optional
offset: int, optional
"""
dtype = ListDtype(element_type=elements.dtype)

result = build_column(
data=None,
dtype=dtype,
mask=mask,
size=size,
offset=offset,
null_count=null_count,
children=(indices, elements),
)

return cast("cudf.core.column.ListColumn", result)


def build_struct_column(
names: Sequence[str],
children: Tuple[ColumnBase, ...],
dtype: Optional[Dtype] = None,
mask: Buffer = None,
size: int = None,
offset: int = 0,
null_count: int = None,
) -> "cudf.core.column.StructColumn":
"""
Build a StructColumn

Parameters
----------
names : list-like
Field names to map to children dtypes
children : tuple

mask: Buffer
Null mask
size: int, optional
offset: int, optional
"""
if dtype is None:
dtype = StructDtype(
fields={name: col.dtype for name, col in zip(names, children)}
)

result = build_column(
data=None,
dtype=dtype,
mask=mask,
size=size,
offset=offset,
null_count=null_count,
children=children,
)

return cast("cudf.core.column.StructColumn", result)


def as_column(
arbitrary: Any,
nan_as_null: bool = None,
Expand Down Expand Up @@ -2200,6 +2272,17 @@ def full(size: int, fill_value: ScalarLike, dtype: Dtype = None) -> ColumnBase:
return ColumnBase.from_scalar(cudf.Scalar(fill_value, dtype), size)


def _cudf_dtype_from_arrow_type(arrow_type: Dtype) -> Dtype:
if pa.types.is_decimal(arrow_type):
return Decimal64Dtype.from_arrow(arrow_type)
elif pa.types.is_struct(arrow_type):
charlesbluca marked this conversation as resolved.
Show resolved Hide resolved
return StructDtype.from_arrow(arrow_type)
elif pa.types.is_list(arrow_type):
return ListDtype.from_arrow(arrow_type)

return arrow_type


def _copy_type_metadata_from_arrow(
arrow_array: pa.array, cudf_column: ColumnBase
) -> ColumnBase:
Expand All @@ -2211,45 +2294,9 @@ def _copy_type_metadata_from_arrow(
* When `arrow_array` is decimal type and `cudf_column` is
Decimal64Dtype, copy precisions.
"""
if pa.types.is_decimal(arrow_array.type) and isinstance(
cudf_column, cudf.core.column.DecimalColumn
):
cudf_column.dtype.precision = arrow_array.type.precision
elif pa.types.is_struct(arrow_array.type) and isinstance(
cudf_column, cudf.core.column.StructColumn
):
base_children = tuple(
_copy_type_metadata_from_arrow(arrow_array.field(i), col_child)
for i, col_child in enumerate(cudf_column.base_children)
)
cudf_column.set_base_children(base_children)
return cudf.core.column.StructColumn(
data=None,
size=cudf_column.base_size,
dtype=StructDtype.from_arrow(arrow_array.type),
mask=cudf_column.base_mask,
offset=cudf_column.offset,
null_count=cudf_column.null_count,
children=base_children,
)
elif pa.types.is_list(arrow_array.type) and isinstance(
cudf_column, cudf.core.column.ListColumn
):
if arrow_array.values and cudf_column.base_children:
base_children = (
cudf_column.base_children[0],
_copy_type_metadata_from_arrow(
arrow_array.values, cudf_column.base_children[1]
),
)
return cudf.core.column.ListColumn(
size=cudf_column.base_size,
dtype=ListDtype.from_arrow(arrow_array.type),
mask=cudf_column.base_mask,
offset=cudf_column.offset,
null_count=cudf_column.null_count,
children=base_children,
)
cudf_column = cudf_column._apply_type_metadata(
_cudf_dtype_from_arrow_type(arrow_array.type)
)

return cudf_column

Expand Down
15 changes: 6 additions & 9 deletions python/cudf/cudf/core/column/decimal.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,13 @@ def __cuda_array_interface__(self):
"Decimals are not yet supported via `__cuda_array_interface__`"
)

def _copy_type_metadata(self: ColumnBase, other: ColumnBase) -> ColumnBase:
"""Copies type metadata from self onto other, returning a new column.
def _apply_type_metadata(
self: "cudf.core.column.DecimalColumn", dtype: Dtype
) -> "cudf.core.column.DecimalColumn":
if isinstance(dtype, Decimal64Dtype):
self.dtype.precision = dtype.precision

In addition to the default behavior, if `other` is also a decimal
column the precision is copied over.
"""
if isinstance(other, DecimalColumn):
other.dtype.precision = self.dtype.precision # type: ignore
# Have to ignore typing here because it misdiagnoses super().
return super()._copy_type_metadata(other) # type: ignore
return self


def _binop_scale(l_dtype, r_dtype, op):
Expand Down
19 changes: 18 additions & 1 deletion python/cudf/cudf/core/column/lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
sort_lists,
)
from cudf._lib.table import Table
from cudf._typing import BinaryOperand
from cudf._typing import BinaryOperand, Dtype
from cudf.core.buffer import Buffer
from cudf.core.column import ColumnBase, as_column, column
from cudf.core.column.methods import ColumnMethodsMixin
Expand Down Expand Up @@ -233,6 +233,23 @@ def __cuda_array_interface__(self):
"Lists are not yet supported via `__cuda_array_interface__`"
)

def _apply_type_metadata(
self: "cudf.core.column.ListColumn", dtype: Dtype
) -> "cudf.core.column.ListColumn":
if isinstance(dtype, ListDtype):
self = column.build_list_column(
indices=self.base_children[0],
elements=self.base_children[1]._apply_type_metadata(
dtype.element_type
),
mask=self.base_mask,
size=self.base_size,
offset=self.offset,
null_count=self.null_count,
)

return self


class ListMethods(ColumnMethodsMixin):
"""
Expand Down
16 changes: 15 additions & 1 deletion python/cudf/cudf/core/column/numerical.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
column,
string,
)
from cudf.core.dtypes import Decimal64Dtype
from cudf.core.dtypes import CategoricalDtype, Decimal64Dtype
from cudf.utils import cudautils, utils
from cudf.utils.dtypes import (
NUMERIC_TYPES,
Expand Down Expand Up @@ -544,6 +544,20 @@ def can_cast_safely(self, to_dtype: DtypeObj) -> bool:

return False

def _apply_type_metadata(self: ColumnBase, dtype: Dtype) -> ColumnBase:
if isinstance(dtype, CategoricalDtype):
self = column.build_categorical_column(
categories=dtype.categories._values,
codes=as_column(self.base_data, dtype=self.dtype),
mask=self.base_mask,
ordered=dtype.ordered,
size=self.size,
offset=self.offset,
null_count=self.null_count,
)

return self

def to_pandas(
self, index: pd.Index = None, nullable: bool = False, **kwargs
) -> "pd.Series":
Expand Down
29 changes: 17 additions & 12 deletions python/cudf/cudf/core/column/struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import pyarrow as pa

import cudf
from cudf.core.column import ColumnBase
from cudf._typing import Dtype
from cudf.core.column import ColumnBase, build_struct_column
from cudf.core.column.methods import ColumnMethodsMixin
from cudf.core.dtypes import StructDtype
from cudf.utils.dtypes import is_struct_dtype


Expand Down Expand Up @@ -111,18 +113,21 @@ def __cuda_array_interface__(self):
"Structs are not yet supported via `__cuda_array_interface__`"
)

def _copy_type_metadata(self: ColumnBase, other: ColumnBase) -> ColumnBase:
"""Copies type metadata from self onto other, returning a new column.

In addition to the default behavior, if `other` is a StructColumns we
rename the fields of `other` to the field names of `self`.
"""
if isinstance(other, cudf.core.column.StructColumn):
other = other._rename_fields(
self.dtype.fields.keys() # type: ignore
def _apply_type_metadata(self: StructColumn, dtype: Dtype) -> StructColumn:
if isinstance(dtype, StructDtype):
self = build_struct_column(
names=dtype.fields.keys(),
children=tuple(
self.base_children[i]._apply_type_metadata(dtype.fields[f])
for i, f in enumerate(dtype.fields.keys())
),
mask=self.base_mask,
size=self.base_size,
offset=self.offset,
null_count=self.null_count,
)
# Have to ignore typing here because it misdiagnoses super().
return super()._copy_type_metadata(other) # type: ignore

return self


class StructMethods(ColumnMethodsMixin):
Expand Down
Loading