API: change Timestamp.strptime to raise NotImplementedError
State: closed

- [x] closes #25016
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Datetime
description: Datetime data dtype

Comments:
> I would much prefer the standard which we already have in datet/time parsing.
> 
> ```
> Timestamp(ts, format=fmt)
> ```
@jreback - So if implemented, we should be able to do `Timestamp(ts, format=fmt)` instead of `Timestamp.strptime(ts, format=fmt)`?


@jreback we inherit `strptime` from `datetime`. Agreed its more pandas-idiomatic just to use `to_datetime`, but I imagine we can handle this method if we add the battery of tests (and discover no major overhauls needed).

I'd be -1 on adding a `format` argument to `Timestamp` directly. I'd rather raise `NotImplementedError` and point people to `to_datetime`.
I did some investigations on local. I am following the idea given by @mroeschke (swap the assignments and the position in constructor). It solves the bug associated with strptime. But it changes Timestamp API that seems unnatural. Can we discuss this point?

The below code give 2 natural (for me) ways to construct a Timestamp. One works and one does not.

```
In [45]: pd.Timestamp(year=2000, month=1, day=2, hour=3, minute=4, second=5, microsecond=6, nanosecond=1, tz='UTC')                                                               
Out[45]: Timestamp('2000-01-02 03:04:05.000006001+0000', tz='UTC')

In [46]: pd.Timestamp(2000, 1, 2, 3, 4, 5, 6, 1, 'UTC')                                                                                                                           
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-46-f9dc555e86e9> in <module>
----> 1 pd.Timestamp(2000, 1, 2, 3, 4, 5, 6, 1, 'UTC')

~/Documents/Projects/pandas-saurav/pandas/_libs/tslibs/timestamps.pyx in pandas._libs.tslibs.timestamps.Timestamp.__new__()
    750                           " tz_convert instead.", FutureWarning)
    751 
--> 752         ts = convert_to_tsobject(ts_input, tz, unit, 0, 0, nanosecond or 0)
    753 
    754         if ts.value == NPY_NAT:

TypeError: an integer is required
```
Agreed, I think it's more natural to have nanosecond come after microsecond positionally instead of tzinfo (that's probably why I added inserted the nanosecond argument in that position in the first place).

But matching the `datetime.datetime` signature ensures we won't get tripped up by internal datetime methods passing `*args` again. We also market Timestamp as being interchangeable with datetime in most cases (and in this case its not).
https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Timestamp.html
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=h1) Report
> Merging [#25124](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e3b095074c5c3b29ec25a994cd204349b872d961?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/25124/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #25124      +/-   ##
==========================================
- Coverage   92.37%   92.37%   -0.01%     
==========================================
  Files         166      166              
  Lines       52408    52415       +7     
==========================================
+ Hits        48412    48418       +6     
- Misses       3996     3997       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.79% <ø> (ø)` | :arrow_up: |
| #single | `42.87% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `88.04% <0%> (-0.1%)` | :arrow_down: |
| [pandas/util/\_decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX2RlY29yYXRvcnMucHk=) | `91.5% <0%> (ø)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.28% <0%> (ø)` | :arrow_up: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.63% <0%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `95.6% <0%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `96.78% <0%> (ø)` | :arrow_up: |
| [pandas/plotting/\_misc.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19taXNjLnB5) | `38.68% <0%> (ø)` | :arrow_up: |
| [pandas/core/groupby/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9nZW5lcmljLnB5) | `87.15% <0%> (ø)` | :arrow_up: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/25124/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.82% <0%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=footer). Last update [e3b0950...fd91d3f](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=h1) Report
> Merging [#25124](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/659e0cae6be2d7ab3370cc7d8ab936bc3ee1b159?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/25124/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #25124   +/-   ##
=======================================
  Coverage   91.73%   91.73%           
=======================================
  Files         173      173           
  Lines       52839    52839           
=======================================
  Hits        48472    48472           
  Misses       4367     4367
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.29% <ø> (ø)` | :arrow_up: |
| #single | `41.72% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=footer). Last update [659e0ca...275c668](https://codecov.io/gh/pandas-dev/pandas/pull/25124?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I would rather raise on .strptime (indifferent on having a format arg though). we don't support this anywhere else.
%z not valid in Python2 strptime(). I guess support for %z is feasible only after we drop support for Py2. 
https://stackoverflow.com/questions/26165659/python-timezone-z-directive-for-datetime-strptime-not-available
Okay @saurav2608, seems like instead making `Timestamp.strptime` raise a `NotImplementedError` with a message pointing toward using `to_datetime` instead is the preferred way to go here.

(Though I think `Timestamp.strptime` could very well be supported in a future PR)
> 
> (Though I think `Timestamp.strptime` could very well be supported in a future PR)

Will you (pandas core team) create a new v0.25 branch? 


No need to worry about that. That will be created when the release is cut (a while from now) and backports are needed.
I can work on the changes to raise `NotImplementedError` next week. I traveling for work and may not find time to focus here. 
I am not able to understand the windows python 2 error. 
Can you try merging master? If it’s for an OO optimization failure just put in a PR to temporarily resolve that

> On Feb 14, 2019, at 9:22 AM, Saurav Chakravorty <notifications@github.com> wrote:
> 
> I am not able to understand the windows python 2 error.
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub <https://github.com/pandas-dev/pandas/pull/25124#issuecomment-463714100>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAlOUX7qaS8uULKVonXOXaQqMRwhJHIlks5vNZtngaJpZM4agQaB>.
> 

William Ayd
william.ayd@icloud.com




Is there some issue with CI? The errors across both my PRs are the same? 
did u merge master?
Yes. I merged master
did i merge master yesterday midday? pls do it again
might be some other issue with excel if u want to have a look
@mroeschke merge when satisified
Thanks @saurav2608!
Is there a reason why we directly changed this behaviour instead of first deprecating it? 
Because we think this is hardly used? (but still, it seems easy to first deprecate it)