Skip to content

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

Merged
merged 23 commits into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b48ee6d
fix: address failing compliance tests in DateArray and TimeArray
tswast Jan 27, 2022
90e1573
fix min/max/median for 2D arrays
tswast Jan 27, 2022
47100f5
fixes except for null contains
tswast Feb 1, 2022
cece518
actually use NaT as 'advertised'
tswast Feb 2, 2022
cc5b178
fix!: use `pandas.NaT` for missing values in dbdate and dbtime dtypes
tswast Jan 27, 2022
cd84754
Merge branch 'issue28-NaT' into issue28-NDArrayBacked2DTests
tswast Feb 2, 2022
f807c6f
Merge remote-tracking branch 'upstream/main' into issue28-NDArrayBack…
tswast Feb 2, 2022
cc713a8
more progress towards compliance
tswast Mar 8, 2022
164101a
address errors in TestMethods
tswast Mar 9, 2022
fb13f0c
fix: correct dtype and interface compliance errors in DateArray
tswast Mar 9, 2022
111a19c
Merge remote-tracking branch 'upstream/main' into issue28-TypeError
tswast Mar 9, 2022
58d3941
add compliance tests to github actions
tswast Mar 9, 2022
7b58a6d
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 9, 2022
8811422
split coverage
tswast Mar 9, 2022
c2b25c8
Merge branch 'issue28-TypeError' of github.com:googleapis/python-db-d…
tswast Mar 9, 2022
9fafe8e
add nox session back
tswast Mar 9, 2022
58da8e0
fix unit session
tswast Mar 9, 2022
d654527
move compliance tests and remove unnecessary test
tswast Mar 10, 2022
69560d2
no need for coverage upload
tswast Mar 10, 2022
ce8d12a
fix coverage
tswast Mar 10, 2022
8c5804f
restore coverage
tswast Mar 10, 2022
c2b0db6
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 10, 2022
ca693cd
Merge branch 'issue28-TypeError' of https://github.com/googleapis/pyt…
gcf-owl-bot[bot] Mar 10, 2022
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
27 changes: 27 additions & 0 deletions .github/workflows/unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,33 @@ jobs:
name: coverage-artifacts
path: .coverage-${{ matrix.python }}

compliance:
Copy link
Contributor

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 because unittest-compliance.yml doesn't exist in templates.

Copy link
Collaborator Author

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 out tests/unit, so this isn't as important.

runs-on: ubuntu-latest
strategy:
matrix:
python: ['3.10']
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python }}
- name: Install nox
run: |
python -m pip install --upgrade setuptools pip wheel
python -m pip install nox
- name: Run compliance tests
env:
COVERAGE_FILE: .coverage-compliance-${{ matrix.python }}
run: |
nox -s compliance
- name: Upload coverage results
uses: actions/upload-artifact@v3
with:
name: coverage-artifacts
path: .coverage-compliance-${{ matrix.python }}

cover:
runs-on: ubuntu-latest
needs:
Expand Down
14 changes: 11 additions & 3 deletions db_dtypes/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import numpy
import pandas
import pandas.api.extensions
from pandas.api.types import is_dtype_equal, is_list_like, pandas_dtype
from pandas.api.types import is_dtype_equal, is_list_like, is_scalar, pandas_dtype

from db_dtypes import pandas_backports

Expand All @@ -31,9 +31,14 @@ class BaseDatetimeDtype(pandas.api.extensions.ExtensionDtype):
names = None

@classmethod
def construct_from_string(cls, name):
def construct_from_string(cls, name: str):
if not isinstance(name, str):
raise TypeError(
f"'construct_from_string' expects a string, got {type(name)}"
)

if name != cls.name:
raise TypeError()
raise TypeError(f"Cannot construct a '{cls.__name__}' from 'another_type'")

return cls()

Expand Down Expand Up @@ -74,6 +79,9 @@ def astype(self, dtype, copy=True):
return super().astype(dtype, copy=copy)

def _cmp_method(self, other, op):
if is_scalar(other) and (pandas.isna(other) or type(other) == self.dtype.type):
other = type(self)([other])

oshape = getattr(other, "shape", None)
if oshape != self.shape and oshape != (1,) and self.shape != (1,):
raise TypeError(
Expand Down
13 changes: 10 additions & 3 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
nox.options.sessions = [
"lint",
"unit",
"compliance",
"cover",
"lint_setup_py",
"blacken",
Expand Down Expand Up @@ -77,7 +78,7 @@ def lint_setup_py(session):
session.run("python", "setup.py", "check", "--restructuredtext", "--strict")


def default(session):
def default(session, tests_path):
# Install all test dependencies, then install this package in-place.

constraints_path = str(
Expand Down Expand Up @@ -106,15 +107,21 @@ def default(session):
"--cov-config=.coveragerc",
"--cov-report=",
"--cov-fail-under=0",
os.path.join("tests", "unit"),
tests_path,
*session.posargs,
)


@nox.session(python=UNIT_TEST_PYTHON_VERSIONS[-1])
def compliance(session):
"""Run the compliance test suite."""
default(session, os.path.join("tests", "compliance"))


@nox.session(python=UNIT_TEST_PYTHON_VERSIONS)
def unit(session):
"""Run the unit test suite."""
default(session)
default(session, os.path.join("tests", "unit"))


@nox.session(python=SYSTEM_TEST_PYTHON_VERSIONS)
Expand Down
68 changes: 68 additions & 0 deletions owlbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,79 @@
new_sessions = """
"lint",
"unit",
"compliance",
"cover",
"""

s.replace(["noxfile.py"], old_sessions, new_sessions)

# Add compliance tests.
s.replace(
["noxfile.py"], r"def default\(session\):", "def default(session, tests_path):"
)
s.replace(["noxfile.py"], r'os.path.join\("tests", "unit"\),', "tests_path,")
s.replace(
["noxfile.py"],
r'''
@nox.session\(python=UNIT_TEST_PYTHON_VERSIONS\)
def unit\(session\):
"""Run the unit test suite."""
default\(session\)
''',
'''
@nox.session(python=UNIT_TEST_PYTHON_VERSIONS[-1])
def compliance(session):
"""Run the compliance test suite."""
default(session, os.path.join("tests", "compliance"))


@nox.session(python=UNIT_TEST_PYTHON_VERSIONS)
def unit(session):
"""Run the unit test suite."""
default(session, os.path.join("tests", "unit"))
''',
)


old_unittest = """
cover:
runs-on: ubuntu-latest
"""

new_unittest = """
compliance:
runs-on: ubuntu-latest
strategy:
matrix:
python: ['3.10']
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python }}
- name: Install nox
run: |
python -m pip install --upgrade setuptools pip wheel
python -m pip install nox
- name: Run compliance tests
env:
COVERAGE_FILE: .coverage-compliance-${{ matrix.python }}
run: |
nox -s compliance
- name: Upload coverage results
uses: actions/upload-artifact@v3
with:
name: coverage-artifacts
path: .coverage-compliance-${{ matrix.python }}

cover:
runs-on: ubuntu-latest
"""

s.replace([".github/workflows/unittest.yml"], old_unittest, new_unittest)

# ----------------------------------------------------------------------------
# Samples templates
# ----------------------------------------------------------------------------
Expand Down
53 changes: 53 additions & 0 deletions tests/compliance/conftest.py
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
47 changes: 47 additions & 0 deletions tests/compliance/date/conftest.py
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()
61 changes: 61 additions & 0 deletions tests/compliance/date/test_date_compliance.py
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):
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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