Skip to content

DOC: don't teach infer_datetime_format in user_guide #50334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 21, 2022
Merged

DOC: don't teach infer_datetime_format in user_guide #50334

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1c1c63
docs: infer_datetime_format() deprecated
Dec 20, 2022
d30ed08
Merge remote-tracking branch 'upstream/main' into doc-date-parser
Dec 20, 2022
056642e
Merge branch 'main' into doc-date-parser
Dec 20, 2022
49c7b24
docs: changes in deprecation msg
Dec 20, 2022
e50682c
Merge branch 'main' into doc-date-parser
ShashwatAgrawal20 Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions doc/source/user_guide/io.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ parse_dates : boolean or list of ints or names or list of lists or dict, default
infer_datetime_format : boolean, default ``False``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, we can't simply remove it. We want to add a deprecation warning before removing it when we enforce the actual deprecation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I asked for it in the issue but no one replied

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phofl should this be Ok 71466b6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - this parameter is now a no-op, so I'd say that it's correct to remove it from the user guide

It still shows in the docstring as deprecated, but I don't think there's any need to teach it in the user guide

Thoughts @phofl ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer mentioning that it is a no op but to keep it in here with a deprecation warning anyway. This should increase visibility, which is helpful with deprecations ( I think). Also keeps us consistent with what we are doing for other parameters in read_csv

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but does a no-op need this much visibility? I think it looks confusing to beginners to read the user guide and find "you can use this parameter to speed things up. Though actually, never mind, it's deprecated, don't use it"

Ok for docstring, it's just for the user guide that I'm questioning the practice

I'll check tomorrow what's been done for other deprecations anyway, and what other libraries do

And sorry @ShashwatAgrawal20 for conflicting instructions here ☺

If ``True`` and parse_dates is enabled for a column, attempt to infer the
datetime format to speed up the processing.

.. deprecated:: 2.0.0
A strict version of this argument is now the default, passing it has no effect.
keep_date_col : boolean, default ``False``
If ``True`` and parse_dates specifies combining multiple columns then keep the
original columns.
Expand Down Expand Up @@ -916,12 +919,10 @@ an exception is raised, the next one is tried:

Note that performance-wise, you should try these methods of parsing dates in order:

1. Try to infer the format using ``infer_datetime_format=True`` (see section below).

2. If you know the format, use ``pd.to_datetime()``:
1. If you know the format, use ``pd.to_datetime()``:
``date_parser=lambda x: pd.to_datetime(x, format=...)``.

3. If you have a really non-standard format, use a custom ``date_parser`` function.
2. If you have a really non-standard format, use a custom ``date_parser`` function.
For optimal performance, this should be vectorized, i.e., it should accept arrays
as arguments.

Expand Down