Skip to content

DOC: Remove old note about default unit #46103

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.

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 3 commits into from
Feb 26, 2022

Conversation

gsganden
Copy link

@gsganden gsganden commented Feb 21, 2022

to_datetime docstring indicates that the default unit is 'ms', but it is 'ns'. This PR simply removes that comment so that there is no need to keep it in sync with the implementation.

  • closes #xxxx (Replace xxxx with the Github issue number) NA
  • Tests added and passed if fixing a bug or adding a new feature NA
  • All code checks passed.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature. NA

@@ -765,8 +765,8 @@ def to_datetime(
unit : str, default 'ns'
The unit of the arg (D,s,ms,us,ns) denote the unit, which is an
integer or float number. This will be based off the origin.
Example, with ``unit='ms'`` and ``origin='unix'`` (the default), this
Copy link
Member

Choose a reason for hiding this comment

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

I am pretty sure this is only referencing origin and not unit

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it could be interpreted either way. I suggest that it's at least potentially confusing and a maintenance risk, so we'd be better off without it.

Copy link
Member

Choose a reason for hiding this comment

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

could change the order to put the "origin=unix (the default)" before "unit=ms"

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, but I suggest that documenting what value is the default in multiple places is asking for trouble in the long run.

@mroeschke mroeschke added Docs Datetime Datetime data dtype labels Feb 22, 2022
@mroeschke mroeschke added this to the 1.5 milestone Feb 22, 2022
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

LGTM. Agreed, (the default) doesn't add much here as the origin default is specified below

@jreback jreback merged commit c8036d3 into pandas-dev:main Feb 26, 2022
@jreback
Copy link
Contributor

jreback commented Feb 26, 2022

thanks @gsganden

yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants