WIP: DatetimeArray+TimedeltaArray #23415

jbrockmendel · 2018-10-30T01:55:46Z

I'll be traveling later this week, so trying to get a few things online before disappearing.

So far this just implements take and concat_same_type. Tests are taken from an older mothballed PR, so are pretty duplicative, need to be cleaned up.

pep8speaks · 2018-10-30T01:55:51Z

Hello @jbrockmendel! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/arrays/__init__.py !
In the file pandas/core/arrays/datetimelike.py, following are the PEP8 issues :

Line 217:5: E265 block comment should start with '# '
Line 218:5: E265 block comment should start with '# '
Line 219:5: E265 block comment should start with '# '
Line 220:5: E265 block comment should start with '# '

There are no PEP8 issues in the file pandas/core/arrays/datetimes.py !
There are no PEP8 issues in the file pandas/core/arrays/period.py !
There are no PEP8 issues in the file pandas/core/arrays/timedeltas.py !
There are no PEP8 issues in the file pandas/core/indexes/datetimelike.py !
There are no PEP8 issues in the file pandas/core/indexes/datetimes.py !
There are no PEP8 issues in the file pandas/core/indexes/period.py !
There are no PEP8 issues in the file pandas/core/indexes/timedeltas.py !
There are no PEP8 issues in the file pandas/core/series.py !
There are no PEP8 issues in the file pandas/tests/arithmetic/conftest.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_datetimelike.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_datetimes.py !
There are no PEP8 issues in the file pandas/tests/dtypes/test_generic.py !
There are no PEP8 issues in the file pandas/util/testing.py !

Comment last updated on November 11, 2018 at 17:44 Hours UTC

codecov · 2018-10-30T02:36:04Z

Codecov Report

Merging #23415 into master will decrease coverage by 0.06%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master   #23415      +/-   ##
==========================================
- Coverage   92.24%   92.17%   -0.07%     
==========================================
  Files         161      161              
  Lines       51278    51223      -55     
==========================================
- Hits        47299    47216      -83     
- Misses       3979     4007      +28

Flag	Coverage Δ
#multiple	`90.61% <84.61%> (-0.02%)`	⬇️
#single	`42.21% <21.15%> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/arrays/period.py	`97.49% <100%> (-0.6%)`	⬇️
pandas/core/arrays/datetimes.py	`97.09% <77.77%> (-1.34%)`	⬇️
pandas/core/arrays/datetimelike.py	`93.43% <78.57%> (-2.48%)`	⬇️
pandas/core/arrays/timedeltas.py	`94.02% <88.88%> (+0.24%)`	⬆️
pandas/io/feather_format.py	`77.14% <0%> (-12.61%)`	⬇️
pandas/io/parquet.py	`73.72% <0%> (-11.04%)`	⬇️
pandas/io/formats/html.py	`90.68% <0%> (-2.47%)`	⬇️
pandas/core/indexes/range.py	`95.77% <0%> (-1.56%)`	⬇️
pandas/core/indexes/period.py	`91.04% <0%> (-1.22%)`	⬇️
... and 77 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3230468...cd63892. Read the comment docs.

jorisvandenbossche · 2018-10-30T13:38:30Z

Quick comment: IMO it is not needed to write tests for basic functioning of eg _concat_same_type or take, this is already covered by the base extension tests.
(for take, there is of course some specific handling of the fill_value, which should be tested specifically for datetime-likes)

…ike_ea

jorisvandenbossche · 2018-11-05T14:15:25Z

@jbrockmendel What is the status of this, or how does it related to the other PRs? Is this still something you plan to continue working on (so worth to review), or not?

jbrockmendel · 2018-11-05T14:59:55Z

This is essentially orthogonal to the other PRs ATM. It has been in hold in part because I expect you would be -1 on implementing any parts of the EA interface without implementing all of them. If that expectation is incorrect, then I would probably add __repr__ and other easily-reviewed needed-eventually methods.

jorisvandenbossche · 2018-11-05T15:08:31Z

My expectation (from the title and original description) was that you would further expand this PR to cover the full EA interface, but that is not the plan?

jbrockmendel · 2018-11-05T16:13:20Z

My expectation was that you would further expand this PR to cover the full EA interface, but that is not the plan?

That was more or less the plan (the "more or less" being that I would have been OK with splitting it into chunks). Besides the trouble with actually implementing this, we've got pieces of this conversation going on in at least four threads, and this is the one I'm most comfortable with letting lie dormant.

…ike_ea

pandas/core/arrays/datetimes.py

…ike_ea

pandas/core/arrays/datetimelike.py

jreback · 2018-11-11T17:48:01Z

pandas/core/arrays/datetimelike.py

+        return cls._simple_new(values, freq=freq)
+
+    def copy(self, deep=False):
+        # TODO: should `deep` determine whether we copy self.asi8?


yes like

values = self.asi8 if deep: values = values.copy() ....

jreback · 2018-11-11T17:49:02Z

pandas/core/arrays/datetimelike.py

+
+    # Following how PeriodArray does this
+    # TODO: ignoring `type`?
+    def view(self, dtype=None, type=None):


why is this so complicated? do we really need this method?

I implemented this because tests were raising AttributeErrors asking for it.

yeah we really can' support .view generally with EA. maybe just leave this on Index.

Really this PR is still sufficiently early in the "WIP" phase it isn't worth spending much time on ATM.

jreback · 2018-11-11T17:50:12Z

pandas/core/arrays/datetimes.py

+
+    @classmethod
+    def _concat_same_type(cls, to_concat):
+        freqs = {x.freq for x in to_concat}


you do this freq dance a few times, maybe a helper function?

I'll take a look.

jreback · 2018-11-11T17:51:18Z

pandas/core/series.py

@@ -4242,6 +4242,9 @@ def _try_cast(arr, take_fast_path):
                                           not (is_iterator(subarr) or
                                           isinstance(subarr, np.ndarray))):
                subarr = construct_1d_object_array_from_listlike(subarr)
+            elif type(subarr).__name__ == "DatetimeArray":


Yah this is a kludge that should have gotten changed to an isinstance(subarr, ABCDatetimeArray) before pushing

jbrockmendel · 2018-11-12T20:24:51Z

The useful parts of this are now a strict subset of #23643. Closing.

Implement concat_same_type, take

e7ecbf8

sinhrks added Datetime Datetime data dtype Timedelta Timedelta data type labels Oct 30, 2018

jorisvandenbossche added ExtensionArray Extending pandas with custom dtypes or arrays. Refactor Internal refactoring of code labels Oct 30, 2018

jorisvandenbossche added this to the 0.24.0 milestone Oct 30, 2018

jbrockmendel added 5 commits October 31, 2018 16:25

implement from_sequence, from_factorized, values_for_factore, copy

e309ab9

rename DatetimeArrayMixin-->DatetimeArray

423a357

implement unique

47689be

commit prelim some I can rebase

dc4a264

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

f92843b

…ike_ea

jorisvandenbossche mentioned this pull request Nov 4, 2018

Datetimelike Array Refactor #23185

Closed

jbrockmendel mentioned this pull request Nov 7, 2018

REF/ENH: Constructors for DatetimeArray/TimedeltaArray #23493

Closed

jbrockmendel added 2 commits November 8, 2018 11:09

update name

92923b2

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

77e9a48

…ike_ea

jbrockmendel commented Nov 8, 2018

View reviewed changes

pandas/core/arrays/datetimes.py Outdated Show resolved Hide resolved

jbrockmendel added 2 commits November 11, 2018 06:46

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

07d4d97

…ike_ea

update imports

cd63892

jreback requested changes Nov 11, 2018

View reviewed changes

jbrockmendel mentioned this pull request Nov 12, 2018

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

Merged

jbrockmendel closed this Nov 12, 2018

jbrockmendel deleted the dlike_ea branch November 12, 2018 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: DatetimeArray+TimedeltaArray #23415

WIP: DatetimeArray+TimedeltaArray #23415

jbrockmendel commented Oct 30, 2018

pep8speaks commented Oct 30, 2018 •

edited

Loading

codecov bot commented Oct 30, 2018 •

edited

Loading

jorisvandenbossche commented Oct 30, 2018

jorisvandenbossche commented Nov 5, 2018

jbrockmendel commented Nov 5, 2018

jorisvandenbossche commented Nov 5, 2018 •

edited

Loading

jbrockmendel commented Nov 5, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

jbrockmendel Nov 11, 2018

jreback Nov 11, 2018

jbrockmendel Nov 11, 2018

jreback Nov 11, 2018

jreback Nov 11, 2018

jbrockmendel Nov 11, 2018

jreback Nov 11, 2018

jbrockmendel Nov 11, 2018

jbrockmendel commented Nov 12, 2018

WIP: DatetimeArray+TimedeltaArray #23415

WIP: DatetimeArray+TimedeltaArray #23415

Conversation

jbrockmendel commented Oct 30, 2018

pep8speaks commented Oct 30, 2018 • edited Loading

Comment last updated on November 11, 2018 at 17:44 Hours UTC

codecov bot commented Oct 30, 2018 • edited Loading

Codecov Report

jorisvandenbossche commented Oct 30, 2018

jorisvandenbossche commented Nov 5, 2018

jbrockmendel commented Nov 5, 2018

jorisvandenbossche commented Nov 5, 2018 • edited Loading

jbrockmendel commented Nov 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Nov 12, 2018

pep8speaks commented Oct 30, 2018 •

edited

Loading

codecov bot commented Oct 30, 2018 •

edited

Loading

jorisvandenbossche commented Nov 5, 2018 •

edited

Loading