PERF: Fixed regression in Series(index=idx) constructor

[TomAugspurger]

From #18496

Special cases empty series construction, since the reindex is not necessary.

xref #18532 (comment)

Setup

data = None
idx = date_range(start=datetime(2015, 10, 26),
                              end=datetime(2016, 1, 1),
                              freq='50s')
dict_data = dict(zip(idx, range(len(idx))))
data = None if data is None else dict_data

benchmark

%timeit out = Series(data, index=idx)

HEAD:

113 µs ± 6.22 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

0.21.0

96.5 µs ± 6.05 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

Master

320 ms ± 11.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

[TomAugspurger]

cc @toobaz

[codecov]

Codecov Report

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

@@            Coverage Diff             @@
##           master   #20865      +/-   ##
==========================================
+ Coverage   91.77%   91.78%   +<.01%     
==========================================
  Files         153      153              
  Lines       49313    49327      +14     
==========================================
+ Hits        45259    45275      +16     
+ Misses       4054     4052       -2

Flag	Coverage Δ
#multiple	90.17% <100%> (ø)	⬆️
#single	41.92% <100%> (+0.03%)	⬆️

Impacted Files	Coverage Δ
pandas/core/series.py	94.03% <100%> (+0.03%)	⬆️
pandas/io/formats/latex.py	100% <0%> (ø)	⬆️
pandas/core/arrays/categorical.py	95.61% <0%> (+0.04%)	⬆️
pandas/util/testing.py	84.59% <0%> (+0.2%)	⬆️

---

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 563a6ad...d8b1312. Read the comment docs.

[jreback]

can you add an asv & a whatsnew (needed?)

[jreback]

rather than doing this here, why not inside _init_dict itself? (you can simply return early if these conditions ar met). you can also return the type, copy parameter as well. ideally like to locate all of a single 'type' code in the same place (dict here)

[toobaz]

I agree - in particular, _init_dict already checks whether data is empty

[TomAugspurger]

Good call on the init_dict. For some reason I thought it had to be in the main Series.init, since _init_dict returns a BlockManager & index, but this seems to work.

In [4]: %timeit out = Series(data, index=idx)
140 µs ± 1.24 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

Shouldn't need a whatsnew since #18496 was in 0.23. I think that was the original slowdown.

And this was caught by the series_methods.SeriesConstructor.time_constructor(None) asv, so we're good there (that's the benchmark I've been running).

[TomAugspurger]

Does d8b1312 change your opinion on putting this stuff in _init_dict? It's getting borderline too messy now.

Series._init_dict sorts the index (keys) (well on non-PY36). This would break

Series(index=['b', 'a'])

Since the user input isn't actually dict-like, it's just going down _init_dict.

[toobaz]

I might be wrong (can't test now, sorry), but a simpler solution could maybe be to replace

pandas/pandas/core/series.py

Line 180 in 4afc756

if data is None:

with

if data in [None, {}]:
    data = []

?

[TomAugspurger]

That won't quiet work since `data` may not be hashable, but I like the thought. Will try a variant.

…

On Mon, Apr 30, 2018 at 8:34 AM, Pietro Battiston ***@***.***> wrote: I might be wrong (can't test now, sorry), but a simpler solution could be to replace https://github.com/pandas-dev/pandas/blob/4afc7563895830d224ac949f571ede f2f069c314/pandas/core/series.py#L180 with if data in [None, {}]: data = [] — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20865 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIk_2hzXwI065z48-lX8D9FfUQJWXks5ttxLQgaJpZM4Trxgq> .

[TomAugspurger]

Ah, no, doesn't quite work since the list isn't broadcast to the length of index. We raise on Series([], index=[0]) since they're different lengths. On Mon, Apr 30, 2018 at 8:39 AM, Tom Augspurger <tom.augspurger88@gmail.com> wrote:

…

That won't quiet work since `data` may not be hashable, but I like the thought. Will try a variant. On Mon, Apr 30, 2018 at 8:34 AM, Pietro Battiston < ***@***.***> wrote: > I might be wrong (can't test now, sorry), but a simpler solution could be > to replace > > https://github.com/pandas-dev/pandas/blob/4afc7563895830d224 > ac949f571edef2f069c314/pandas/core/series.py#L180 > > with > > if data in [None, {}]: > data = [] > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#20865 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABQHIk_2hzXwI065z48-lX8D9FfUQJWXks5ttxLQgaJpZM4Trxgq> > . >

[toobaz]

Ah, no, doesn't quite work since the list isn't broadcast to the length of index.

Right!

I thought at least replacing data = {} with data = na_value_for_dtype(dtype) would alleviate the problem for the data=None case, but I'm getting some infinite recursion when collecting tests which I'm unable to explain.

[TomAugspurger]

Merging in 2 hours if there are no further comments. I think we're just choosing between the two least-bad outcomes here.

[jreback]

let me look again

[jreback]

lgtm. thanks (I believe you said this was after 0.22 so no whatsnew needed)

[TomAugspurger]

Right, slowdown was only on master.