fix: support converting empty `time` Series to pyarrow Array by tswast · Pull Request #11 · googleapis/python-db-dtypes-pandas

tswast · 2021-09-27T16:00:24Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #10 🦕

tswast · 2021-09-27T16:01:04Z

Currently getting pyarrow.lib.ArrowNotImplementedError: Unsupported cast from double to time64 using function cast_time64 in the empty time Series test.

tswast · 2021-09-27T16:53:13Z

I'm a bit surprised this doesn't happen for date. I'll check with plain pandas timedelta64 to see what it does with to_numpy() for an empty series.

tswast · 2021-09-27T16:58:51Z

Seems to work fine on latest pandas. 🤔

In [1]: import pandas

In [2]: pandas.__version__
Out[2]: '1.4.0.dev0+502.g019d782633'

In [3]: s = pandas.Series([], dtype='timedelta64[ns]')

In [4]: import pyarrow

In [5]: a = pyarrow.array(s)

In [6]: a
Out[6]:
<pyarrow.lib.DurationArray object at 0x7fb98813bd60>
[]

In [7]: s.to_numpy()
Out[7]: array([], dtype='timedelta64[ns]')

tswast · 2021-09-27T20:37:34Z

    def __arrow_array__(self, type=None):
        return pyarrow.array(
-            self.to_numpy(), type=type if type is not None else pyarrow.time64("ns"),
+            self.to_numpy(dtype="object"),


This might have performance implications, but it does seem to prevent the cast to float64 for empty arrays. Also, the dtype seems to be object whenever there are any values in the array, anyway.

I approached this by adding the missing to_numpy() for pandas <1, that just uses astype('object')

Sweet. That seems to have done the trick. I copied the implementation from your PR https://github.com/googleapis/python-db-dtypes-pandas/pull/9/files#diff-1956943f14005805ef968dfc37c26fe3eee995786f62a1c66dee5a29d9b1a251R111

jimfulton · 2021-09-28T01:55:53Z

+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import datetime as dt
+
+import pandas
+import pyarrow
+import pytest
+
+# To register the types.
+import db_dtypes  # noqa
+
+
+@pytest.mark.parametrize(
+    ("series", "expected"),
+    (
+        (pandas.Series([], dtype="date"), pyarrow.array([], type=pyarrow.date32())),
+        (
+            pandas.Series([None, None, None], dtype="date"),
+            pyarrow.array([None, None, None], type=pyarrow.date32()),
+        ),
+        (
+            pandas.Series(
+                [dt.date(2021, 9, 27), None, dt.date(2011, 9, 27)], dtype="date"
+            ),
+            pyarrow.array(
+                [dt.date(2021, 9, 27), None, dt.date(2011, 9, 27)],
+                type=pyarrow.date32(),
+            ),
+        ),
+        (
+            pandas.Series(
+                [dt.date(1677, 9, 22), dt.date(1970, 1, 1), dt.date(2262, 4, 11)],
+                dtype="date",
+            ),
+            pyarrow.array(
+                [dt.date(1677, 9, 22), dt.date(1970, 1, 1), dt.date(2262, 4, 11)],
+                type=pyarrow.date32(),
+            ),
+        ),
+        (pandas.Series([], dtype="time"), pyarrow.array([], type=pyarrow.time64("ns"))),
+        (
+            pandas.Series([None, None, None], dtype="time"),
+            pyarrow.array([None, None, None], type=pyarrow.time64("ns")),
+        ),
+        (
+            pandas.Series(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_999)], dtype="time"
+            ),
+            pyarrow.array(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_999)],
+                type=pyarrow.time64("ns"),
+            ),
+        ),
+        (
+            pandas.Series(
+                [
+                    dt.time(0, 0, 0, 0),
+                    dt.time(12, 30, 15, 125_000),
+                    dt.time(23, 59, 59, 999_999),
+                ],
+                dtype="time",
+            ),
+            pyarrow.array(
+                [
+                    dt.time(0, 0, 0, 0),
+                    dt.time(12, 30, 15, 125_000),
+                    dt.time(23, 59, 59, 999_999),
+                ],
+                type=pyarrow.time64("ns"),
+            ),
+        ),
+    ),
+)
+def test_to_arrow(series, expected):
+    array = pyarrow.array(series)
+    assert array.equals(expected)
+
+
+@pytest.mark.parametrize(
+    ("series", "expected"),
+    (
+        (pandas.Series([], dtype="date"), pyarrow.array([], type=pyarrow.date64())),
+        (
+            pandas.Series([None, None, None], dtype="date"),
+            pyarrow.array([None, None, None], type=pyarrow.date64()),
+        ),
+        (
+            pandas.Series(
+                [dt.date(2021, 9, 27), None, dt.date(2011, 9, 27)], dtype="date"
+            ),
+            pyarrow.array(
+                [dt.date(2021, 9, 27), None, dt.date(2011, 9, 27)],
+                type=pyarrow.date64(),
+            ),
+        ),
+        (
+            pandas.Series(
+                [dt.date(1677, 9, 22), dt.date(1970, 1, 1), dt.date(2262, 4, 11)],
+                dtype="date",
+            ),
+            pyarrow.array(
+                [dt.date(1677, 9, 22), dt.date(1970, 1, 1), dt.date(2262, 4, 11)],
+                type=pyarrow.date64(),
+            ),
+        ),
+        (pandas.Series([], dtype="time"), pyarrow.array([], type=pyarrow.time32("ms"))),
+        (
+            pandas.Series([None, None, None], dtype="time"),
+            pyarrow.array([None, None, None], type=pyarrow.time32("ms")),
+        ),
+        (
+            pandas.Series(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_000)], dtype="time"
+            ),
+            pyarrow.array(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_000)],
+                type=pyarrow.time32("ms"),
+            ),
+        ),
+        (
+            pandas.Series(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_999)], dtype="time"
+            ),
+            pyarrow.array(
+                [dt.time(0, 0, 0, 0), None, dt.time(23, 59, 59, 999_999)],
+                type=pyarrow.time64("us"),
+            ),
+        ),
+        (
+            pandas.Series(
+                [
+                    dt.time(0, 0, 0, 0),
+                    dt.time(12, 30, 15, 125_000),
+                    dt.time(23, 59, 59, 999_999),
+                ],
+                dtype="time",
+            ),
+            pyarrow.array(
+                [
+                    dt.time(0, 0, 0, 0),
+                    dt.time(12, 30, 15, 125_000),
+                    dt.time(23, 59, 59, 999_999),
+                ],
+                type=pyarrow.time64("us"),
+            ),
+        ),
+    ),
+)
+def test_to_arrow_w_arrow_type(series, expected):
+    array = pyarrow.array(series, type=expected.type)
+    assert array.equals(expected)


Your tests are nicer than mine. :)

Thanks! They did catch a bug with empty arrays, so I'm glad I wrote them

plamut

Looks good. I just can't say about the possible performance impact of self.to_numpy(dtype="object").

Are we going to use that conditionally for pandas<1 as Jim suggested?

tswast · 2021-09-28T19:48:32Z

I just can't say about the possible performance impact of self.to_numpy(dtype="object").

Thankfully I think this approach with using an extension dtype leaves us room for optimization. We might even switch to backing everything with pyarrow arrays instead of numpy arrays someday.

Are we going to use that conditionally for pandas<1 as Jim suggested?

Yes, that seems to be the right approach. Just pushed a commit.

tswast · 2021-09-29T14:04:24Z

-
-
-@for_date_and_time
-def test_date___arrow__array__(dtype):


Removed because it's redundant with the new test_arrow.py tests.

🤖 I have created a release \*beep\* \*boop\* --- ## 0.1.0 (2021-09-29) ### Features * add `time` and `date` dtypes ([f104171](https://www.github.com/googleapis/python-db-dtypes-pandas/commit/f10417111642e8f5f4b9af790367af930d15a056)) ### Bug Fixes * support converting empty `time` Series to pyarrow Array ([#11](https://www.github.com/googleapis/python-db-dtypes-pandas/issues/11)) ([7675b15](https://www.github.com/googleapis/python-db-dtypes-pandas/commit/7675b157feb842628fa731cc6a472aa9e6b92903)) * support Pandas 0.24 ([#8](https://www.github.com/googleapis/python-db-dtypes-pandas/issues/8)) ([e996883](https://www.github.com/googleapis/python-db-dtypes-pandas/commit/e996883bc9c76fe5f593e9c19a9d2a1c13501f5e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 27, 2021

product-auto-label Bot added the api: bigquery Issues related to the googleapis/python-db-dtypes-pandas API. label Sep 27, 2021

fix: support converting empty time Series to pyarrow Array

e793ec6

tswast force-pushed the issue10-arrow-coverage branch from d225329 to e793ec6 Compare September 27, 2021 16:01

tswast changed the title ~~fix: support constucting empty time Series~~ fix: support converting empty time Series to pyarrow Array Sep 27, 2021

tswast marked this pull request as ready for review September 27, 2021 20:35

tswast requested a review from a team September 27, 2021 20:35

use object dtype for time numpy array

e08f7fc

tswast commented Sep 27, 2021

View reviewed changes

tswast requested review from jimfulton and plamut September 27, 2021 20:38

tswast mentioned this pull request Sep 27, 2021

test: No warnings #9

Merged

4 tasks done

jimfulton reviewed Sep 28, 2021

View reviewed changes

plamut reviewed Sep 28, 2021

View reviewed changes

backport to_numpy

281afb9

tswast requested a review from plamut September 28, 2021 20:48

tswast added 2 commits September 28, 2021 17:02

Merge remote-tracking branch 'upstream/main' into issue10-arrow-coverage

1613bb6

remove redundant test

63b946c

tswast commented Sep 29, 2021

View reviewed changes

plamut approved these changes Sep 29, 2021

View reviewed changes

tswast merged commit 7675b15 into main Sep 29, 2021

tswast deleted the issue10-arrow-coverage branch September 29, 2021 16:31

Navigation Menu

fix: support converting empty `time` Series to pyarrow Array - #11

fix: support converting empty `time` Series to pyarrow Array#11
tswast merged 5 commits into
mainfrom
issue10-arrow-coverage

tswast commented Sep 27, 2021 •

edited

Loading

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast Sep 27, 2021

Uh oh!

jimfulton Sep 28, 2021

Uh oh!

tswast Sep 28, 2021

Uh oh!

jimfulton Sep 28, 2021

Uh oh!

tswast Sep 28, 2021

Uh oh!

plamut left a comment

Uh oh!

tswast commented Sep 28, 2021

Uh oh!

tswast Sep 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tswast commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast commented Sep 27, 2021

Uh oh!

tswast Sep 27, 2021

Choose a reason for hiding this comment

Uh oh!

jimfulton Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

tswast Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

jimfulton Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

tswast Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

plamut left a comment

Choose a reason for hiding this comment

Uh oh!

tswast commented Sep 28, 2021

Uh oh!

tswast Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tswast commented Sep 27, 2021 •

edited

Loading