8000 Implement _most_ of the EA interface for DTA/TDA by jbrockmendel · Pull Request #23643 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

Implement _most_ of the EA interface for DTA/TDA #23643

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

Merged
merged 11 commits into from
Nov 14, 2018
Merged

Implement _most_ of the EA interface for DTA/TDA #23643

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
52e4d6b
implement most of the rest of EA interface
jbrockmendel Nov 12, 2018
c2bcd80
Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…
jbrockmendel Nov 12, 2018
0ce0a7c
Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…
jbrockmendel Nov 12, 2018
45c8161
implement some tests for take, concat_same_type
jbrockmendel Nov 12, 2018
75f6944
Fixup typo, implement more tests
jbrockmendel Nov 12, 2018
0fb5029
whitespace fixup
jbrockmendel Nov 12, 2018
1a781ab
suggested edits
jbrockmendel Nov 12, 2018
ccbffe4
Merge remote-tracking branch 'upstream/master' into jbrockmendel-dlik…
TomAugspurger Nov 13, 2018
eceebc7
REF: Simplify concat
TomAugspurger Nov 14, 2018
a6065cc
copy
TomAugspurger Nov 14, 2018
3cb072e
deduplicate copy
TomAugspurger Nov 14, 2018
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
Prev Previous commit
Next Next commit
whitespace fixup
  • Loading branch information
@jbrockmendel
jbrockmendel committed Nov 12, 2018
commit 0fb5029a176f9802d4e4e4b41be32bb197eefa12
36 changes: 18 additions & 18 deletions pandas/tests/arrays/test_datetimelike.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,60 +58,60 @@ def timedelta_index(request):

class SharedTests(object):
index_cls = None

def test_take(self):
Copy link
Member

Choose a reason for hiding this comment

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

We should not need to add such basic tests I think, as those are covered by the base Extension tests (we should of course test datetime specific aspects).

Is there anything in this test not tested by the base tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

The class-specific tests have tests for invalid fill_values

data = np.arange(100, dtype='i8')
np.random.shuffle(data)

idx = self.index_cls._simple_new(data, freq='D')
arr = self.array_cls(idx)

takers = [1, 4, 94]
result = arr.take(takers)
expected = idx.take(takers)

tm.assert_index_equal(self.index_cls(result), expected)

takers = np.array([1, 4, 94])
result = arr.take(takers)
expected = idx.take(takers)

tm.assert_index_equal(self.index_cls(result), expected)

def test_take_fill(self):
data = np.arange(10, dtype='i8')

idx = self.index_cls._simple_new(data, freq='D')
arr = self.array_cls(idx)

result = arr.take([-1, 1], allow_fill=True, fill_value=None)
assert result[0] is pd.NaT

result = arr.take([-1, 1], allow_fill=True, fill_value=np.nan)
assert result[0] is pd.NaT

result = arr.take([-1, 1], allow_fill=True, fill_value=pd.NaT)
assert result[0] is pd.NaT

with pytest.raises(ValueError):
arr.take([0, 1], allow_fill=True, fill_value=2)

with pytest.raises(ValueError):
arr.take([0, 1], allow_fill=True, fill_value=2.0)

with pytest.raises(ValueError):
arr.take([0, 1], allow_fill=True,
fill_value=pd.Timestamp.now().time)

def test_concat_same_type(self):
Copy link
Member

Choose a reason for hiding this comment

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

As I think also commented on one of the previous PRs that started doing this, I don't think we should test _concat_same_type here directly. It is already tested by the base extension tests and by all the tests that actually use it under the hood.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yah, most of these tests were salvaged from one of those older PRs. I don't see much downside to having the tests, but am pretty happy to pawn this decision/PR off on Tom

Copy link
Contributor
@TomAugspurger TomAugspurger Nov 13, 2018 •
edited
Loading

We still want the fail cases (different dtypes) here.

At this point, we should be able to simplify core/dtypes/concat.py::concat_datetimetz and DatetimeIndex._concat_same_dtype right? I'll take a look.

data = np.arange(10, dtype='i8')

idx = self.index_cls._simple_new(data, freq='D').insert(0, pd.NaT)
arr = self.array_cls(idx)

result = arr._concat_same_type([arr[:-1], arr[1:], arr])
expected = idx._concat_same_dtype([idx[:-1], idx[1:], idx], None)

tm.assert_index_equal(self.index_cls(result), expected)


Expand Down
0