-
Notifications
You must be signed in to change notification settings - Fork 6
fix: correct TypeError and comparison issues discovered in DateArray compliance tests #79
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
Changes from 17 commits
b48ee6d
90e1573
47100f5
cece518
cc5b178
cd84754
f807c6f
cc713a8
164101a
fb13f0c
111a19c
58d3941
7b58a6d
8811422
c2b25c8
9fafe8e
58da8e0
d654527
69560d2
ce8d12a
8c5804f
c2b0db6
ca693cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
# Copyright 2022 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 pandas | ||
import pytest | ||
|
||
|
||
@pytest.fixture(params=["ffill", "bfill"]) | ||
def fillna_method(request): | ||
""" | ||
Parametrized fixture giving method parameters 'ffill' and 'bfill' for | ||
Series.fillna(method=<method>) testing. | ||
|
||
See: | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/conftest.py | ||
""" | ||
return request.param | ||
|
||
|
||
@pytest.fixture | ||
def na_value(): | ||
return pandas.NaT | ||
|
||
|
||
@pytest.fixture | ||
def na_cmp(): | ||
""" | ||
Binary operator for comparing NA values. | ||
|
||
Should return a function of two arguments that returns | ||
True if both arguments are (scalar) NA for your type. | ||
|
||
See: | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/conftest.py | ||
and | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/test_datetime.py | ||
""" | ||
|
||
def cmp(a, b): | ||
return a is pandas.NaT and a is b | ||
|
||
return cmp |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
# Copyright 2022 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 | ||
|
||
import numpy | ||
import pytest | ||
|
||
from db_dtypes import DateArray, DateDtype | ||
|
||
|
||
@pytest.fixture | ||
def data(): | ||
return DateArray( | ||
numpy.arange( | ||
datetime.datetime(1900, 1, 1), | ||
datetime.datetime(2099, 12, 31), | ||
datetime.timedelta(days=731), | ||
dtype="datetime64[ns]", | ||
) | ||
) | ||
|
||
|
||
@pytest.fixture | ||
def data_missing(): | ||
"""Length-2 array with [NA, Valid] | ||
|
||
See: | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/conftest.py | ||
""" | ||
return DateArray([None, datetime.date(2022, 1, 27)]) | ||
|
||
|
||
@pytest.fixture | ||
def dtype(): | ||
return DateDtype() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# Copyright 2022 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. | ||
""" | ||
Tests for extension interface compliance, inherited from pandas. | ||
|
||
See: | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/decimal/test_decimal.py | ||
and | ||
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/extension/test_period.py | ||
""" | ||
|
||
import datetime | ||
|
||
from pandas.tests.extension import base | ||
|
||
import db_dtypes | ||
|
||
|
||
class TestDtype(base.BaseDtypeTests): | ||
pass | ||
|
||
|
||
class TestInterface(base.BaseInterfaceTests): | ||
pass | ||
|
||
|
||
class TestConstructors(base.BaseConstructorsTests): | ||
pass | ||
|
||
|
||
class TestReshaping(base.BaseReshapingTests): | ||
pass | ||
|
||
|
||
class TestGetitem(base.BaseGetitemTests): | ||
def test_take_na_value_other_date(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be helpful to have an additional comment in the test to clarify the expected behaviour There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Would have been nice if pandas had a bit more of an explanatory comment, but I'll try and deduce why this test was added in pandas-dev/pandas#20814. Perhaps it's not even necessary in general? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed, as most other extension dtype tests don't have it. |
||
arr = db_dtypes.DateArray( | ||
[datetime.date(2022, 3, 8), datetime.date(2022, 3, 9)] | ||
) | ||
result = arr.take( | ||
[0, -1], allow_fill=True, fill_value=datetime.date(1969, 12, 31) | ||
) | ||
expected = db_dtypes.DateArray( | ||
[datetime.date(2022, 3, 8), datetime.date(1969, 12, 31)] | ||
) | ||
self.assert_extension_array_equal(result, expected) | ||
|
||
|
||
class TestMissing(base.BaseMissingTests): | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively, you could pull this into a separate file called
unittest-compliance.yml
and remove the customizations in owlbot.py for.github/workflows/unittest.yml
. Owlbot shouldn't overwrite the changes becauseunittest-compliance.yml
doesn't exist in templates.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give that a try! I was thinking mostly about having this count towards
coverage
, but I see that specifically calls outtests/unit
, so this isn't as important.