PERF: add shortcut to Timestamp constructor
State: closed

- [X] closes #30543 
- [X] tests added 1 / passed 1
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

This implements a shortcut in the Timestamp constructor to cut down on processing if Timestamp is passed. We still need to check if the timezone was passed correctly. Then, if a Timestamp was passed, and there is no timezone, we just return that same Timestamp.
A test is added to check that the Timestamp is still the same object.

PR for timedelta to be added once I confirm that this is the approach we want to go with.


Labels:
name: Performance
description: Memory or execution speed performance

Comments:
Simply adding a shortcut breaks date_range in an unintuitive way (see test `TestDatetimeIndexTimezones.test_dti_construction_ambiguous_endpoint`). I'll look deeper into this today to see why date_range no longer takes into account daylight savings with my fix.
Hello @AlexKirko! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:







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

##### Comment last updated at 2020-01-24 09:24:16 UTC
I suppose I should also move this to whatsnew for version 1.1.0?
**Update**: moved it since we already have the 1.0.0 release candidate.
when you say cast? are you doing an astype or some sort of loop? can i show the original example perf issue
When I say "cast" mean a `timeit` loop of `pd.Timestamp(ts)` in the python console. Something along the lines of:
```
ts = pd.Timestamp(100)
timeit(lambda: pd.Timestamp(ts), number=10000000)
```
@jreback Thanks for suggesting designing and running more tests. I went ahead and did that.

**TLDR:** depending on the usage conditions, the shortcut can speed up conversion to Timestamp to very different degrees. We speed things up about:
1. \>10 000 times in a `timeit` call in the console (testing done previously).
2. ~7 times in a `%timeit` call in Jupyter Notebook
3. ~30% when using `pd.Series.apply`
4. Nothing when using `pd.Series.astype`.

In addition:
1. Hoisting the check to the very top of `__new__`speeds up loop performance by about 20% and doesn't speed up anything else.
2. Replacing all the `is not None` checks with `not any` on a list comprehension slows down loop performance by about 100%, slows down `apply` a bit, and doesn't impact `astype`.

I think that implementing the shortcut is definitely worth it. As much as we might prefer it to not be true, some people will always loop when there is a better solution available (and sometimtes it's not available). I don't think moving the check to the very top of the function is worth it, as that is very ugly, but using expanded `is not None` gives a large performance boost in loops, so I think we should keep it.

What do you think?

The performance tests code and the results are below.

<details>

If we run the original perf issue code on master:
```
IN:
ts = pd.Timestamp('2020')
%timeit pd.Timestamp(ts)
OUT:
2.17 µs ± 168 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
```
And on the current commit of the PR branch (`bf08cc8`):
```
IN:
ts = pd.Timestamp('2020')
%timeit pd.Timestamp(ts)
OUT:
372 ns ± 31 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
```
So with original issue code, the shortcut runs about 7 times faster.
This is, of course, the worst possible case, but loops might be used only by somebody just starting with `pandas` and Python in general.

Let's test this further.
On master:
```
IN:
t_ser = pd.Series(range(100000))
t_ser = t_ser.astype('<M8[ns]')
%timeit t_ser.apply(lambda x: pd.Timestamp(x))
%timeit t_ser.astype('<M8[ns]')
OUT:
634 ms ± 25.4 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
146 µs ± 2.73 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```
And on the current commit of the PR branch (`bf08cc8`):
```
IN:
t_ser = pd.Series(range(100000))
t_ser = t_ser.astype('<M8[ns]')
%timeit t_ser.apply(lambda x: pd.Timestamp(x))
%timeit t_ser.astype('<M8[ns]')
OUT:
464 ms ± 28.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
151 µs ± 4.07 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```
So we save about 85% of time on a loop, 30% of time on `apply`, and nothing on `astype`. My guess is that `astype` has its own shortcut for when the user tries to cast a Series into its own dtype.
I also hoisted the check to the very top of the constructor and ran this again:
```
IN:
ts = pd.Timestamp('2020')
%timeit pd.Timestamp(ts)
OUT:
298 ns ± 18 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)


IN:
t_ser = pd.Series(range(100000))
t_ser = t_ser.astype('<M8[ns]')
%timeit t_ser.apply(lambda x: pd.Timestamp(x))
%timeit t_ser.astype('<M8[ns]')
OUT:
451 ms ± 23 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
160 µs ± 8.24 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```
So hoisting appears to help on loops.
And also ran it on commit `4c9eb70` (the solution with `not any(arg is not None for arg in _non_ts_attributes)`:
```
IN:
ts = pd.Timestamp('2020')
%timeit pd.Timestamp(ts)
OUT:
623 ns ± 25.3 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

IN:
t_ser = pd.Series(range(100000))
t_ser = t_ser.astype('<M8[ns]')
%timeit t_ser.apply(lambda x: pd.Timestamp(x))
%timeit t_ser.astype('<M8[ns]')
OUT:
482 ms ± 18.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
154 µs ± 3.65 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)
```
With list comprehensions, this runs about two times slower in a loop, and a bit slower in lambda. My guess is that since the comprehension doesn't depend on the value converted, it gets optimized away.

</details>

Also pinging @jbrockmendel @WillAyd 
These testing results should make it easier to decide on what we want to implement here.
@jreback 
Done.
@jreback 
All green, should be ready to merge.
lgtm. can you rebase, ping on green.
@jreback 
Rebased, all green.
Also cleaned up the conflicts with my previour 2 PRs. Don't need one of the `dateutil` xfails now.
thanks @AlexKirko very nice! keep em coming