Improved docstring and return type hints for `to_datetime`
State: closed

- [x] closes https://github.com/pandas-dev/pandas/issues/17973

Improved (still not ideal) fix for #42229 (datetime parsing behaviour in case of multiple time offsets due to daylight-savings in timezones) following https://github.com/pandas-dev/pandas/pull/42244 

See also https://github.com/pandas-dev/pandas/issues/42229#issuecomment-870009579


Labels:
name: Docs
name: Timezones
description: Timezone data dtype

Comments:
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@jreback do you have any additional response to the above comment ? otherwise we can probably proceed with merging "as is"
closing as stale, if you want to continue working, please ping.
@jreback thanks for the ping. I made changes in my branch but I do not see them here - I guess that you'll have to open the PR to see them ? 

I tried to update the documentation of the "utc" parameter so that it is easier to understand. I added the link that @mroeschke suggested in here (rather than in a notes section, but I guess we could also do both).

Tomorrow I'll try to add a few examples showing the subtelties explained in the utc parameter doc.
 
Thanks @mroeschke ! I'll push a few doc mods again and let you know.
@jreback and @mroeschke  : all set.

Instead of trying to squeeze a complex explanation in the arguments and outputs definitions, I rather created a proper extended summary.

I also fixed the return type hints and added several examples. These examples show that the current behaviour is quite complex, but at least now users can see it :) . Let me know what you think
Looks like the `CI / Checks` failed (which will check this docstring). Also recommend to keep type hinting in a separate PR so limit the review scope.
Thanks @mroeschke for the detailed review ! I'll have a look when possible, probably next week.
@mroeschke I did all the easy ones, thanks it was very relevant !

Now we're left with three of your comments that are more or less related to a single idea of simplification, if I get them well :

 - moving the "detailed summary" in the "returns" section
 - moving some of the "utc" parameter description in the "examples" section
 - "cleaning up the prose" in the "examples section

My problem is as follows: the current behaviour of this function is extremely confusing and may even seems erratic, for people discovering it. So ideally of course we would change/fix the function. That could make the documentation a lot simpler, and I would agree to most of your comments above tending to simplification.

However it is not in the scope of this PR to fix the function, as it would require far more maintenance team agreement/consensus. As a consequence I tried to document all the situations, so that at least we progress from "apparently erratic" to "documented". Since the cases to cover are many, this does not fit in the usual "returns" section, this is why I moved it in the detailed summary where we have more room. Similar, this is why I have created many examples, one for each different situation (only one per case, not more). Finally, since the cause of all apparent problems come from the difficulty to understand the "utc" parameter because it makes several things at once, I documented it very extensively (also linking to the more general pandas timezone documentation as you suggested).

The goal is not to keep such a detailed documentation forever, but to use it as a step towards a future code change leading to function simplification (and doc simplification). Does it make sense ?

That being said, I am of course still completely open to specific changes suggestions, thanks for taking the time to consider this PR !
`to_datetime` has a lot of arguments and accepts a lot of array-like and scalar-like argument so naturally this function will be quite complex (as dates are quite complex) and ideally all the interactions of inputs + arguments should be documented.

You are welcome to submit and issue per behavior you think needs changing, but note they may likely be API changes and may be difficult to deprecate + remove as other users may have been relying on this behavior for a while.


Thanks @mroeschke. Indeed it will be quite a challenge to tackle this through a deprecation cycle ! I'll open a corresponding issue when this PR is ok.

So for now, let's stick to the doc improvement. I left the detailed version for now, so that the "traps" are visible and documented. Do you think that we should invite another reviewer for a "last pass" ?

Hi @jreback , gentle reminder about this PR - I guess that we are not far at all from closing here.
> Hi @jreback , gentle reminder about this PR - I guess that we are not far at all from closing here.

need to rebase and address all comments
Thanks a lot @jreback for the review ! 

I moved the detailed summary to the "Notes" section. I modified the output to compress it as you suggested. I answered the comments about the examples.

> can you organize the examples, there is a lot going on and its hard to tell what is what.
> maybe put some comments to denote different sections of the examples.

I would be happy to structure this more. Do we have an example somewhere else in pandas where the examples have different subsections ? I'll try to find some

Hi @mroeschke  and @jreback , I took a few hours again to update this PR.

 - I reduced the comments about the utc parameter and replaced them with a link to the examples section
 - I made several sub-sections in the examples
 - I made the examples section look nicer
 - I tested my documentation with local build `python make.py --single pandas.to_datetime` : it looks quite good ! 

I might be over-optimistic here, but this could be actually matching your expectations :)

I let you have a look (please look at the locally built doc, it is far easier to follow) and tell me what you think !
Have a nice xmas break!

thanks for taking the time to review ! I took into account all of your comments @mroeschke , let me know if the result still needs modification.

I also realize that I did not write an entry in what's new. I see in 1.1.0 what's new that "clarified documentation" can be inserted in the bugfix section (see https://pandas.pydata.org/docs/whatsnew/v1.1.0.html?highlight=clarified#missing), so added something similar, let me know.


@smarie if you can update and merge master
Sorry for the delay @jreback @mroeschke . I updated the branch and removed the whats new entry.
Build seems ok except for a few that seem unrelated to this PR
thanks @smarie if you can rebase one more time pls.

also if you can go over the text again and be consistent about using backticks vs class refs (e.g. i think we prefer to use ```:class:`Timestamp` ``` over ``` ``Timestamp`` ``` (and same for `datetime.datetime`); you'l likely have to render this locally to make sure things come out ok. (this is also ok as a followup, but since need rebase anyhow....)


cc @mroeschke if comments
from the doc build: 

> /home/runner/work/pandas/pandas/pandas/core/tools/datetimes.py:docstring of pandas.core.tools.datetimes.to_datetime:72: WARNING: Unexpected indentation.
Thanks, It seems that this is now fixed @jreback . I fixed as much as possible the various sphinx roles everywhere, using explicit `:class:`, `:mod:` or `:const:` whenever it was relevant.
The remaining error is "/home/runner/work/pandas/pandas/pandas/core/ops/array_ops.py
  /home/runner/work/pandas/pandas/pandas/core/ops/array_ops.py:98:17 - error: Unnecessary "cast" call; type is already "dtype[Unknown]" (reportUnnecessaryCast)"

so it seems unrelated
thanks @smarie very nice!
Thanks very much @jreback and @mroeschke for your patience and careful review.