BUG: Fix #25959 - Don't call .array in DatetimeLikeArrayMixin's map #25964

ThomasKluiters · 2019-04-02T20:47:15Z

closes Series.apply fails when the series is a timezone aware datetime #25959
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

In Series.apply we call .array on returning the result of DatetimeLikeArrayMixin map, which causes an error.

codecov · 2019-04-02T21:21:38Z

Codecov Report

Merging #25964 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25964      +/-   ##
==========================================
+ Coverage   91.69%   91.72%   +0.03%     
==========================================
  Files         174      174              
  Lines       50749    50754       +5     
==========================================
+ Hits        46534    46556      +22     
+ Misses       4215     4198      -17

Flag	Coverage Δ
#multiple	`90.23% <100%> (+0.04%)`	⬆️
#single	`41.69% <50%> (+0.38%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/series.py	`93.67% <100%> (ø)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/compat/__init__.py	`93.54% <0%> (-0.4%)`	⬇️
pandas/plotting/_style.py	`76.92% <0%> (-0.26%)`	⬇️
pandas/plotting/_misc.py	`38.23% <0%> (-0.23%)`	⬇️
pandas/core/frame.py	`97.02% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.6% <0%> (-0.11%)`	⬇️
pandas/plotting/_converter.py	`63.66% <0%> (-0.06%)`	⬇️
pandas/core/groupby/generic.py	`88.96% <0%> (-0.05%)`	⬇️
pandas/core/dtypes/dtypes.py	`96.65% <0%> (-0.02%)`	⬇️
... and 21 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 e5d15b2...36429a0. Read the comment docs.

mroeschke

Some comments and also needs a whatsnew entry in 0.25

pandas/tests/indexes/datetimes/test_datetime.py

pandas/core/arrays/datetimelike.py

ThomasKluiters · 2019-04-04T09:24:04Z

Some comments and also needs a whatsnew entry in 0.25

Added!

pandas/tests/resample/test_datetime_index.py

doc/source/whatsnew/v0.25.0.rst

pandas/tests/resample/test_datetime_index.py

pep8speaks · 2019-04-04T17:32:49Z

Hello @ThomasKluiters! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-05-18 19:09:18 UTC

doc/source/whatsnew/v0.25.0.rst

ThomasKluiters · 2019-04-06T09:23:56Z

I don't know why the tests are failing - locally all seems fine

Edit - updating from master seemed to have resolved the issue.

ThomasKluiters · 2019-04-08T16:22:20Z

@jreback @mroeschke I resolved my issues, could you have a look please?

pandas/core/series.py

TomAugspurger · 2019-04-09T12:28:03Z

Can you investigate what happens if we don’t cal tolist? What tests fail?

…

On Apr 9, 2019, at 06:00, Thomas Kluiters ***@***.***> wrote: @ThomasKluiters commented on this pull request. In pandas/core/series.py: > @@ -3688,7 +3688,11 @@ def f(x): if len(mapped) and isinstance(mapped[0], Series): from pandas.core.frame import DataFrame - return DataFrame(mapped.tolist(), index=self.index) + # GH 25959 we don't need to call tolist + # if we've been given an ExtensionArray + if not isinstance(mapped, ExtensionArray): It was initially there - I do not know, I resolved your problem though. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

ThomasKluiters · 2019-04-09T13:52:33Z

Can you investigate what happens if we don’t cal tolist? What tests fail?
…
On Apr 9, 2019, at 06:00, Thomas Kluiters @.***> wrote: @ThomasKluiters commented on this pull request. In pandas/core/series.py: > @@ -3688,7 +3688,11 @@ def f(x): if len(mapped) and isinstance(mapped[0], Series): from pandas.core.frame import DataFrame - return DataFrame(mapped.tolist(), index=self.index) + # GH 25959 we don't need to call tolist + # if we've been given an ExtensionArray + if not isinstance(mapped, ExtensionArray): It was initially there - I do not know, I resolved your problem though. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Removing the tolist will make this test fail:
Relate to issue #2316
`____________________________________________________________________________ TestSeriesAggregate.test_with_nested_series ____________________________________________________________________________

self = <pandas.tests.series.test_apply.TestSeriesAggregate object at 0x12ebc31d0>
datetime_series = 2000-01-03 -1.347822
2000-01-04 0.739531
2000-01-05 0.230190
2000-01-06 -0.397356
2000-01-07 -0.128090
200...2-08 -1.054427
2000-02-09 0.360638
2000-02-10 -0.639306
2000-02-11 -0.517083
Freq: B, Name: ts, dtype: float64

def test_with_nested_series(self, datetime_series):
    # GH 2316
    # .agg with a reducer and a transform, what to do
    result = datetime_series.apply(lambda x: Series(
        [x, x ** 2], index=['x', 'x^2']))
    expected = DataFrame({'x': datetime_series,
                          'x^2': datetime_series ** 2})

  tm.assert_frame_equal(result, expected)

E AssertionError: DataFrame are different
E
E DataFrame shape mismatch
E [left]: (30, 1)
E [right]: (30, 2)
`

TomAugspurger · 2019-04-10T01:42:33Z

So are we using tolist to get an array from a series? Maybe call pd.array on the object instead of tolist?

…25959)

ThomasKluiters · 2019-04-10T07:42:21Z

So are we using tolist to get an array from a series? Maybe call pd.array on the object instead of tolist?

pd.array seems to be the best solution, it removes the need to check for an EA. Any suggestions on how I can fix the conflict best in the whatsnew?

TomAugspurger · 2019-04-10T13:18:25Z

Actually I think we have a better function than pd.array. It’s something like extract_array in internals.

…

On Apr 10, 2019, at 02:42, Thomas Kluiters ***@***.***> wrote: So are we using tolist to get an array from a series? Maybe call pd.array on the object instead of tolist? pd.array seems to be the best solution, it removes the need to check for an EA. Any suggestions on how I can fix the conflict best in the whatsnew? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

ThomasKluiters · 2019-04-10T19:43:45Z

extract_array

extract_array does not work I'm afraid, it will fail the same test

pandas/core/series.py

jreback · 2019-04-20T17:04:00Z

can you merge master

ThomasKluiters · 2019-04-21T20:06:25Z

can you merge master

Done!

jreback · 2019-04-28T18:43:23Z

can you merge master and ping on green.

…x-25959

jreback · 2019-05-07T01:24:10Z

you have an isort error; can you merge master and fix up. ping on green.

…x-25959

ThomasKluiters · 2019-05-07T15:06:03Z

you have an isort error; can you merge master and fix up. ping on green.

Sorry, but I can't seem to find what's wrong with the build - it seems to be failing at /doc/source/reference/frame.rst - "DataFrame" has no attribute "to_panel".

WillAyd · 2019-05-07T15:06:38Z

Should be fixed after #26308

jreback · 2019-05-12T20:55:43Z

can you merge master

…x-25959

jreback

small comments, ping on green.

pandas/core/series.py

jreback · 2019-05-18T14:41:31Z

linting error:

./pandas/core/series.py(3708,80): error E501: line too long (82 > 79 characters)
Bash exited with code '1'.

jreback · 2019-05-18T14:41:39Z

ping on green.

pandas/tests/series/test_apply.py

ThomasKluiters · 2019-05-19T14:22:25Z

@jreblack ping

jreback · 2019-05-19T17:09:44Z

thanks @ThomasKluiters

mroeschke requested changes Apr 2, 2019

View reviewed changes

pandas/tests/indexes/datetimes/test_datetime.py Outdated Show resolved Hide resolved

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

gfyoung added Bug Timezones Timezone data dtype labels Apr 3, 2019

jreback requested changes Apr 4, 2019

View reviewed changes

pandas/tests/resample/test_datetime_index.py Outdated Show resolved Hide resolved

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved

pandas/tests/resample/test_datetime_index.py Outdated Show resolved Hide resolved

mroeschke reviewed Apr 4, 2019

View reviewed changes

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved

TomAugspurger reviewed Apr 8, 2019

View reviewed changes

pandas/core/series.py Outdated Show resolved Hide resolved

jreback added this to the 0.25.0 milestone Apr 9, 2019

ThomasKluiters force-pushed the fix-25959 branch from 47a19af to 2803f11 Compare April 10, 2019 07:35

BUG: Fix apply on series backed by datetime aware values (pandas-dev#…

223f59f

…25959)

ThomasKluiters force-pushed the fix-25959 branch from 2803f11 to 223f59f Compare April 10, 2019 07:41

TomAugspurger reviewed Apr 10, 2019

View reviewed changes

pandas/core/series.py Show resolved Hide resolved

jreback removed this from the 0.25.0 milestone Apr 20, 2019

Merge branch 'master' into fix-25959

eb37d2e

jreback added this to the 0.25.0 milestone Apr 28, 2019

jreback approved these changes Apr 28, 2019

View reviewed changes

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

4ab1005

…x-25959

ThomasKluiters added 2 commits May 7, 2019 14:17

Fix isort issue

339b155

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

440a2b3

…x-25959

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

8f6054e

…x-25959

jreback requested changes May 16, 2019

View reviewed changes

pandas/core/series.py Show resolved Hide resolved

Use constructor_expanddim instead of DataFrame

4ad0bae

jreback requested changes May 18, 2019

View reviewed changes

pandas/tests/series/test_apply.py Show resolved Hide resolved

Add regression test and fix PEP

36429a0

jreback approved these changes May 19, 2019

View reviewed changes

jreback merged commit 6cf3b5c into pandas-dev:master May 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix #25959 - Don't call .array in DatetimeLikeArrayMixin's map #25964

BUG: Fix #25959 - Don't call .array in DatetimeLikeArrayMixin's map #25964

ThomasKluiters commented Apr 2, 2019 •

edited

Loading

codecov bot commented Apr 2, 2019 •

edited

Loading

mroeschke left a comment

ThomasKluiters commented Apr 4, 2019

pep8speaks commented Apr 4, 2019 •

edited

Loading

ThomasKluiters commented Apr 6, 2019 •

edited

Loading

ThomasKluiters commented Apr 8, 2019

TomAugspurger commented Apr 9, 2019 via email

ThomasKluiters commented Apr 9, 2019 •

edited

Loading

TomAugspurger commented Apr 10, 2019

ThomasKluiters commented Apr 10, 2019

TomAugspurger commented Apr 10, 2019 via email

ThomasKluiters commented Apr 10, 2019

jreback commented Apr 20, 2019

ThomasKluiters commented Apr 21, 2019

jreback commented Apr 28, 2019

jreback commented May 7, 2019

ThomasKluiters commented May 7, 2019

WillAyd commented May 7, 2019

jreback commented May 12, 2019

jreback left a comment

jreback commented May 18, 2019

jreback commented May 18, 2019

ThomasKluiters commented May 19, 2019

jreback commented May 19, 2019

BUG: Fix #25959 - Don't call .array in DatetimeLikeArrayMixin's map #25964

BUG: Fix #25959 - Don't call .array in DatetimeLikeArrayMixin's map #25964

Conversation

ThomasKluiters commented Apr 2, 2019 • edited Loading

codecov bot commented Apr 2, 2019 • edited Loading

Codecov Report

mroeschke left a comment

Choose a reason for hiding this comment

ThomasKluiters commented Apr 4, 2019

pep8speaks commented Apr 4, 2019 • edited Loading

Comment last updated at 2019-05-18 19:09:18 UTC

ThomasKluiters commented Apr 6, 2019 • edited Loading

ThomasKluiters commented Apr 8, 2019

TomAugspurger commented Apr 9, 2019 via email

ThomasKluiters commented Apr 9, 2019 • edited Loading

TomAugspurger commented Apr 10, 2019

ThomasKluiters commented Apr 10, 2019

TomAugspurger commented Apr 10, 2019 via email

ThomasKluiters commented Apr 10, 2019

jreback commented Apr 20, 2019

ThomasKluiters commented Apr 21, 2019

jreback commented Apr 28, 2019

jreback commented May 7, 2019

ThomasKluiters commented May 7, 2019

WillAyd commented May 7, 2019

jreback commented May 12, 2019

jreback left a comment

Choose a reason for hiding this comment

jreback commented May 18, 2019

jreback commented May 18, 2019

ThomasKluiters commented May 19, 2019

jreback commented May 19, 2019

ThomasKluiters commented Apr 2, 2019 •

edited

Loading

codecov bot commented Apr 2, 2019 •

edited

Loading

pep8speaks commented Apr 4, 2019 •

edited

Loading

ThomasKluiters commented Apr 6, 2019 •

edited

Loading

ThomasKluiters commented Apr 9, 2019 •

edited

Loading