Skip to content

_ensure_type should use issubclass #32416

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

_ensure_type should use issubclass #32416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8bcd0f
BUG: Replace isinstance with issubclass in _ensure_type (#31925)
ericchansen Mar 3, 2020
5ddbea9
TST: Added tests for pandas object ensure type (#31925)
ericchansen Mar 3, 2020
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
2 changes: 1 addition & 1 deletion pandas/core/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def _ensure_type(self: T, obj) -> T:

Used by type checkers.
"""
assert isinstance(obj, type(self)), type(obj)
assert issubclass(type(obj), type(self)), type(obj)
Copy link
Member

Choose a reason for hiding this comment

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

The purpose of ensure_type is to ensure that we have a type and not None for methods with an inplace parameter where the return type is Optional. does this work?

Suggested change
assert issubclass(type(obj), type(self)), type(obj)
assert obj is not None

Copy link
Member

Choose a reason for hiding this comment

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

will probably also need to change
def _ensure_type(self: T, obj) -> T: to def _ensure_type(self, obj: Optional[T]) -> T:

Copy link
Contributor

Choose a reason for hiding this comment

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

If @simonjayhawkins 's suggestion works, I think that would be the mest fool-proof, though I'm not sure I understand the issue.

Another thing we probably should consider is never return multiple types, so e.g. no Optional[FrameOrSeries] until we get to python 3.8 as the lowest python version. If we did that _ensure_type could be pulled from the code base.

return obj


Expand Down
38 changes: 38 additions & 0 deletions pandas/tests/base/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import pytest

from pandas.core.base import PandasObject

pandas_object = PandasObject()


class SubclassPandasObject(PandasObject):
pass


subclass_pandas_object = SubclassPandasObject()


@pytest.mark.parametrize("other_object", [pandas_object, subclass_pandas_object])
def test_pandas_object_ensure_type(other_object):
pandas_object = PandasObject()
assert pandas_object._ensure_type(other_object)


def test_pandas_object_ensure_type_for_same_object():
pandas_object_a = PandasObject()
pandas_object_b = pandas_object_a
assert pandas_object_a._ensure_type(pandas_object_b)


class OtherClass:
pass


other_class = OtherClass()


@pytest.mark.parametrize("other_object", [other_class])
def test_pandas_object_ensure_type_for_false(other_object):
pandas_object = PandasObject()
with pytest.raises(AssertionError):
assert pandas_object._ensure_type(other_object)