API: consistent array for datetime-like ExtensionArrays #23593

jorisvandenbossche · 2018-11-09T10:36:38Z

Already makes the __array__ for the datetimelike EAs consistent with each other and with Series/Index (for the default case of not specifying a dtype)

pep8speaks · 2018-11-09T10:36:41Z

Hello @jorisvandenbossche! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 05, 2019 at 20:26 Hours UTC

TomAugspurger · 2018-11-09T13:21:54Z

isort failure:

Check import format using isort
ERROR: /home/travis/build/pandas-dev/pandas/pandas/tests/extension/base/interface.py Imports are incorrectly sorted.

jorisvandenbossche · 2018-11-09T13:25:57Z

Yeah, I will still need to get used to it :-)
Do you have it as a commit hook?

TomAugspurger · 2018-11-09T13:29:14Z

I have for dask-ml. I'm experimenting with it in pandas today.

install pre-commit
paste this to .pre-commit-config.yaml

-   repo: https://github.com/pre-commit/mirrors-isort
    rev: 'f35773e46d096de5c45365f1a47eeeef36fc83ed'  # Use the revision sha / tag you want to point at
    hooks:
    -   id: isort

-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.0.0  # Use the ref you want to point at
    hooks:
    -   id: flake8

run pre-commit install

I'll open an issue suggesting that for the docs if it works well.

codecov · 2018-11-09T14:05:45Z

Codecov Report

Merging #23593 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #23593      +/-   ##
==========================================
+ Coverage   92.37%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52379    52377       -2     
==========================================
  Hits        48386    48386              
+ Misses       3993     3991       -2

Flag	Coverage Δ
#multiple	`90.8% <100%> (ø)`	⬆️
#single	`43.01% <77.77%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/arrays/timedeltas.py	`88.24% <ø> (+0.05%)`	⬆️
pandas/core/arrays/datetimelike.py	`97.86% <100%> (+0.01%)`	⬆️
pandas/core/arrays/period.py	`98.54% <100%> (ø)`	⬆️
pandas/core/arrays/datetimes.py	`97.84% <100%> (-0.01%)`	⬇️
pandas/util/testing.py	`88.09% <0%> (+0.09%)`	⬆️

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 f074abe...4120586. Read the comment docs.

jbrockmendel · 2018-11-09T14:14:52Z

pandas/tests/arrays/test_datetimelike.py

@@ -126,6 +126,24 @@ def test_int_properties(self, datetime_index, propname):

        tm.assert_numpy_array_equal(result, expected)

+    def test_array(self, datetime_index):


while you're at it, can you make sure there are analogous tests for the DTI/TDI/PI index classes?

OK, I only have to see then a bit how that overlaps with your PR then

Do you know if there are any existing tests on this somewhere (for the index classes)? Not directly finding something

jbrockmendel · 2018-11-09T14:22:20Z

Implement view while at it?
Wrong button, woops

jorisvandenbossche · 2018-11-09T14:22:47Z

Implement view while at it?

On EAs? Is there a reason to do that?

jbrockmendel · 2018-11-09T18:28:29Z

On EAs? Is there a reason to do that?

IIRC from other branches we'll need to do it eventually. Since I don't have the precise reason on hand, it's reasonable to wait until actually needed.

TomAugspurger · 2018-11-09T22:53:18Z

From #23569 (comment)

You questioned whether we should implement np.array(period_array, dtype=int). You haven't done that right?

I'm not really clear what's best here. Do we publicly document exactly what an ordinal is, beyond just saying that it's an integer (though maybe that's enough justify making int)?

jreback · 2018-11-11T16:21:27Z

@jbrockmendel can you rebase this.

TomAugspurger · 2018-11-11T17:26:38Z

This one is from Joris :)

jreback · 2018-11-11T17:34:36Z

ok then @jorisvandenbossche lgtm.

jreback · 2018-11-23T03:19:25Z

@jorisvandenbossche can you rebase

jorisvandenbossche · 2018-11-23T10:53:18Z

Fixed the linting error.

But, we still need to decide on #23593 (comment): should np.array(period_array, dtype='int') work?
PeriodIndex currently does this (but also without making a copy, which I think we should change to conform to numpy behaviour).

I'm not really clear what's best here. Do we publicly document exactly what an ordinal is, beyond just saying that it's an integer (though maybe that's enough justify making int)?

I don't really care that much. If "power users" want to work with the integers, I think the asi8 attribute is more explicit to use? (and is already public)

TomAugspurger · 2018-11-23T11:49:06Z

Sounds good to me.

…

On Fri, Nov 23, 2018 at 4:53 AM Joris Van den Bossche < ***@***.***> wrote: Fixed the linting error. But, we still need to decide on #23593 (comment) <#23593 (comment)>: should np.array(period_array, dtype='int') work? PeriodIndex currently does this (but also without making a copy, which I think we should change to conform to numpy behaviour). I'm not really clear what's best here. Do we publicly document exactly what an ordinal is, beyond just saying that it's an integer (though maybe that's enough justify making int)? I don't really care that much. If "power users" want to work with the integers, I think the asi8 attribute is more explicit to use? (and is already public) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#23593 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHItg6Sd05Rh7_p7EkVCv2N2HuOThlks5ux9OlgaJpZM4YWVyz> .

jreback · 2018-11-23T12:36:10Z

yeah this is ok

pandas/core/arrays/period.py

jreback · 2018-12-28T20:35:55Z

need to merge master on this

jreback · 2019-01-03T01:13:29Z

@jorisvandenbossche @TomAugspurger @jbrockmendel should rebase this and see where things are.

pandas/tests/arrays/test_datetimelike.py

jreback · 2019-01-04T14:01:26Z

@jorisvandenbossche can you merge master

pandas/tests/extension/base/interface.py

jorisvandenbossche · 2019-01-04T14:09:42Z

@jreback any other comments?

@TomAugspurger fine to merge this? There is some overlap with your other PR on __array__, but also still some content that is not in there.

TomAugspurger · 2019-01-04T14:13:51Z

Yep, this is fine. I'd prefer this goes in first.

jreback

see my comments

pandas/tests/extension/base/interface.py

pandas/core/arrays/period.py

jreback · 2019-01-04T16:38:08Z

pandas/core/arrays/datetimelike.py

+        # used for Timedelta/DatetimeArray, overwritten by PeriodArray
+        if is_object_dtype(dtype):
+            return np.array(list(self), dtype=object)
+        return self._data


I would raise if dtype not in [None, object]

jorisvandenbossche · 2019-01-04T16:46:55Z

I would raise if dtype not in [None, object]

Why? Eg for Datetime/TimedeltaArray, now np.asarray(.., dtype='M8[ns] / 'int64') now works as expected.
In case of PeriodArray, numpy will already raise an error if it cannot convert to the specified dtype.

jreback · 2019-01-04T16:48:36Z

Why? Eg for Datetime/TimedeltaArray, now np.asarray(.., dtype='M8[ns] / 'int64') now works as expected.
In case of PeriodArray, numpy will already raise an error if it cannot convert to the specified dtype.

outside of the specified dtypes, you must raise for a dtype if you are not passing to np.array() which I don't think you are.

but this is really easy to answer. provide tests that use non-valid dtypes and show that it raises.

jorisvandenbossche · 2019-01-04T16:50:48Z

provide tests that use non-valid dtypes and show that it raises.

This is already tested for PeriodArray: https://github.com/pandas-dev/pandas/pull/23593/files#diff-fff2a1331dd7864425d7581505b95e58R622

TomAugspurger · 2019-01-04T17:00:46Z

but this is really easy to answer. provide tests that use non-valid dtypes and show that it raises.

And that can't be a base test since we don't know what is and isn't valid for subtypes.

Agreed with @jorisvandenbossche that NumPy should handle the exception raising if the values aren't valid for the requested dtype.

jreback · 2019-01-05T18:49:38Z

can you merge master

…e-ea-array-protocol

TomAugspurger · 2019-01-05T20:26:29Z

Merged master and

Updated DatetimeArray.__array__ to change the default dtype to object for DatetimeTZ
Fixed a lint error in the tests.

jreback · 2019-01-05T21:17:44Z

thanks @jorisvandenbossche and @TomAugspurger

…ev#23593)

API: consistent __array__ for datetime-like ExtensionArrays

8ebe597

jorisvandenbossche added the ExtensionArray Extending pandas with custom dtypes or arrays. label Nov 9, 2018

jorisvandenbossche added this to the 0.24.0 milestone Nov 9, 2018

jorisvandenbossche mentioned this pull request Nov 9, 2018

BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index #23524

Merged

4 tasks

temp override DatetimelikeIndex.__array__

6e8eaff

fix isort

9506fef

jbrockmendel reviewed Nov 9, 2018

View reviewed changes

jbrockmendel closed this Nov 9, 2018

jbrockmendel reopened this Nov 9, 2018

jorisvandenbossche added 2 commits November 16, 2018 14:30

Merge remote-tracking branch 'upstream/master' into ea-array-protocol

bade9e4

fixes for changes in master

8c68e2a

jorisvandenbossche force-pushed the ea-array-protocol branch from d77b823 to 8c68e2a Compare November 16, 2018 14:03

jorisvandenbossche added 2 commits November 23, 2018 11:34

Merge remote-tracking branch 'upstream/master' into ea-array-protocol

d8f24fd

fix import

39bae20

jreback reviewed Dec 18, 2018

View reviewed changes

pandas/core/arrays/period.py Show resolved Hide resolved

jorisvandenbossche added 4 commits January 3, 2019 09:11

Merge remote-tracking branch 'upstream/master' into ea-array-protocol

eb6c8bd

add tests for other dtypes

c5404d7

clean-up merge

d9ab5bf

linting

b51c4b3

jreback requested changes Jan 3, 2019

View reviewed changes

pandas/tests/arrays/test_datetimelike.py Show resolved Hide resolved

jreback requested changes Jan 4, 2019

View reviewed changes

pandas/tests/extension/base/interface.py Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into ea-array-protocol

79908d1

jreback requested changes Jan 4, 2019

View reviewed changes

pandas/tests/extension/base/interface.py Show resolved Hide resolved

small clean-up tests

1112420

jreback requested changes Jan 4, 2019

View reviewed changes

add tests for case of specifying its own dtype

e284aad

Merge remote-tracking branch 'upstream/master' into jorisvandenbossch…

4120586

…e-ea-array-protocol

jreback approved these changes Jan 5, 2019

View reviewed changes

jreback merged commit 280a88f into pandas-dev:master Jan 5, 2019

jorisvandenbossche deleted the ea-array-protocol branch January 6, 2019 21:08

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

API: consistent __array__ for datetime-like ExtensionArrays (pandas-d…

e74c07c

…ev#23593)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

API: consistent __array__ for datetime-like ExtensionArrays (pandas-d…

deef8e4

…ev#23593)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: consistent array for datetime-like ExtensionArrays #23593

API: consistent array for datetime-like ExtensionArrays #23593

jorisvandenbossche commented Nov 9, 2018

pep8speaks commented Nov 9, 2018 •

edited

Loading

TomAugspurger commented Nov 9, 2018

jorisvandenbossche commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

codecov bot commented Nov 9, 2018 •

edited

Loading

jbrockmendel Nov 9, 2018

jorisvandenbossche Nov 9, 2018

jorisvandenbossche Nov 9, 2018 •

edited

Loading

jbrockmendel commented Nov 9, 2018 •

edited

Loading

jorisvandenbossche commented Nov 9, 2018

jbrockmendel commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018 •

edited

Loading

jreback commented Nov 11, 2018

TomAugspurger commented Nov 11, 2018

jreback commented Nov 11, 2018

jreback commented Nov 23, 2018

jorisvandenbossche commented Nov 23, 2018

TomAugspurger commented Nov 23, 2018 via email

jreback commented Nov 23, 2018

jreback commented Dec 28, 2018

jreback commented Jan 3, 2019

jreback commented Jan 4, 2019

jorisvandenbossche commented Jan 4, 2019

TomAugspurger commented Jan 4, 2019

jreback left a comment

jreback Jan 4, 2019

jorisvandenbossche commented Jan 4, 2019

jreback commented Jan 4, 2019

jorisvandenbossche commented Jan 4, 2019

TomAugspurger commented Jan 4, 2019

jreback commented Jan 5, 2019

TomAugspurger commented Jan 5, 2019

jreback commented Jan 5, 2019

		@@ -126,6 +126,24 @@ def test_int_properties(self, datetime_index, propname):

		tm.assert_numpy_array_equal(result, expected)

		def test_array(self, datetime_index):

API: consistent __array__ for datetime-like ExtensionArrays #23593

API: consistent __array__ for datetime-like ExtensionArrays #23593

Conversation

jorisvandenbossche commented Nov 9, 2018

pep8speaks commented Nov 9, 2018 • edited Loading

Comment last updated on January 05, 2019 at 20:26 Hours UTC

TomAugspurger commented Nov 9, 2018

jorisvandenbossche commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

codecov bot commented Nov 9, 2018 • edited Loading

Codecov Report

jbrockmendel Nov 9, 2018

Choose a reason for hiding this comment

jorisvandenbossche Nov 9, 2018

Choose a reason for hiding this comment

jorisvandenbossche Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

jbrockmendel commented Nov 9, 2018 • edited Loading

jorisvandenbossche commented Nov 9, 2018

jbrockmendel commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018 • edited Loading

jreback commented Nov 11, 2018

TomAugspurger commented Nov 11, 2018

jreback commented Nov 11, 2018

jreback commented Nov 23, 2018

jorisvandenbossche commented Nov 23, 2018

TomAugspurger commented Nov 23, 2018 via email

jreback commented Nov 23, 2018

jreback commented Dec 28, 2018

jreback commented Jan 3, 2019

jreback commented Jan 4, 2019

jorisvandenbossche commented Jan 4, 2019

TomAugspurger commented Jan 4, 2019

jreback left a comment

Choose a reason for hiding this comment

jreback Jan 4, 2019

Choose a reason for hiding this comment

jorisvandenbossche commented Jan 4, 2019

jreback commented Jan 4, 2019

jorisvandenbossche commented Jan 4, 2019

TomAugspurger commented Jan 4, 2019

jreback commented Jan 5, 2019

TomAugspurger commented Jan 5, 2019

jreback commented Jan 5, 2019

API: consistent array for datetime-like ExtensionArrays #23593

API: consistent array for datetime-like ExtensionArrays #23593

pep8speaks commented Nov 9, 2018 •

edited

Loading

codecov bot commented Nov 9, 2018 •

edited

Loading

jorisvandenbossche Nov 9, 2018 •

edited

Loading

jbrockmendel commented Nov 9, 2018 •

edited

Loading

TomAugspurger commented Nov 9, 2018 •

edited

Loading