Skip to content

bpo-37914:Support total_hours() and total_minutes() in class timedelta#15481

Closed
hongweipeng wants to merge 2 commits into
python:masterfrom
hongweipeng:issue37914
Closed

bpo-37914:Support total_hours() and total_minutes() in class timedelta#15481
hongweipeng wants to merge 2 commits into
python:masterfrom
hongweipeng:issue37914

Conversation

@hongweipeng

@hongweipeng hongweipeng commented Aug 25, 2019

Copy link
Copy Markdown
Contributor

@nanjekyejoannah nanjekyejoannah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was a consesus reached to add these accessors @pganssle ? Looks like the discussion here : https://bugs.python.org/issue37914 was not in favor of adding them. Am I missing something ?

@aeros

aeros commented Aug 25, 2019

Copy link
Copy Markdown
Contributor

From Paul Ganssle:

The reason there is no hours or minutes is that the normalized form of timedelta doesn't have those components. It would be inconsistent to have hours and minutes give the total duration of the timedelta in the chosen units while .days and .seconds return just the component of the normalized form.

What's really being asked for here are total_hours() and total_minutes() methods, and when that has come up in the past (including recently on the python-dev mailing list), we've largely decided that the "divide by the units you want" idiom is sufficient (and in fact better in that you can choose any arbitrary units rather than just the ones that have specific names).

Since this has been discussed extensively, both on the ML and on the issue page from one of the datetime experts, I'm going to label to this PR as invalid. I'll leave the final decision for closing it to @pganssle though.

Edit: The bpo issue should also be closed.

@pganssle

Copy link
Copy Markdown
Member

Thanks @aeros167 and @nanjekyejoannah for the comments and @serhiy-storchaka for closing the issue.

@hongweipeng Sorry if there was some miscommunication about this, we are indeed going to stick with the approach of dividing by timedelta objects to get out "total" units, but thank you for your contribution. I hope this does not discourage you from making further patches.

@hongweipeng hongweipeng deleted the issue37914 branch September 2, 2019 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants