Deprecate relative_time in favor of time_since and time_until #86136

rlippmann · 2023-01-18T03:42:54Z

Proposed change

Add future and depth support to the relative_time template function:

make relative_time support future datetimes as well as more precision
future support is useful for timers. A simple filter is easier for the end user instead of having to write long and complex jinja for templates to parse it out
Depth parameter name was taken from Django: https://github.com/django/django/blob/main/django/utils/timesince.py, it specifies how many actual units of time one desires, i.e. 2 days (depth = 1) vs 1 hour 45 minutes (depth = 2). Currently relative_time just rounds to the highest unit (i.e. would only show 2 days no matter what)
the future = True isn’t necessary, it’s just there to prevent this from being a breaking change

with these changes, an end user can simply do:
relative_time(sensor.timer_name,future=true, depth = whatever) to create a human readable version of time remaining

currently to do this the user needs to deal with datetime objects and strptime and then chop up the resulting string.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: [(https://github.com/Deprecate relative_time in favor of time_since()/time_until() home-assistant.io#25631)]

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

emontnemery · 2023-01-18T08:34:17Z

@rlippmann Please improve the PR description to give some background and motivation to why this is useful. Also, is a numerical depth a well known way to specify the precision of time intervals? If it's not, maybe the new parameter should be a string instead which accepts "years", "months" etc.?

rlippmann · 2023-01-18T09:38:07Z

@rlippmann Please improve the PR description to give some background and motivation to why this is useful. Also, is a numerical depth a well known way to specify the precision of time intervals? If it's not, maybe the new parameter should be a string instead which accepts "years", "months" etc.?

Updated

rlippmann · 2023-01-25T03:27:26Z

Should I close this?

haydndup · 2023-02-22T08:18:51Z

This functionality would be great - I'm trying to create some local calendar reminders, and need things like in 2 weeks which I just found out wasn't possible with the existing relative_time implementation.

rlippmann · 2023-03-01T17:48:59Z

This functionality would be great - I'm trying to create some local calendar reminders, and need things like in 2 weeks which I just found out wasn't possible with the existing relative_time implementation.

Well, I’m not sure how to get this merged. Or why it’s been sitting here for months…

@frenck can you advise?

rlippmann · 2023-03-03T17:57:11Z

This functionality would be great - I'm trying to create some local calendar reminders, and need things like in 2 weeks which I just found out wasn't possible with the existing relative_time implementation.

@haydndup maybe if you reviewed this it could finally get merged….

haydndup · 2023-03-06T08:17:26Z

@haydndup maybe if you reviewed this it could finally get merged….

@rlippmann, I reviewed and approved the changes and it looks good to me. I'm not an approving reviewer, so would still need additional approvals from the HA team. I left some minor suggestions in home-assistant/home-assistant.io#25631.

rlippmann · 2023-03-06T08:36:25Z

@haydndup maybe if you reviewed this it could finally get merged….

@rlippmann, I reviewed and approved the changes and it looks good to me. I'm not an approving reviewer, so would still need additional approvals from the HA team. I left some minor suggestions in home-assistant/home-assistant.io#25631.

Thanks! Now if only I can find an approving reviewer to approve, lol. This would make life so much easier for writing templates for timers.

epenet · 2023-03-06T08:39:56Z

Merge branch 'dev' into relative_time

Please do not merge dev unnecessarily into your branch unless there is a merge conflict or you are in need of new features from dev branch. It creates a useless run on CI.

anoblet · 2023-04-21T22:50:25Z

I would find the feature very useful as well.

disforw · 2023-07-10T11:25:49Z

homeassistant/helpers/template.py

@@ -2095,15 +2095,23 @@ def today_at(hass: HomeAssistant, time_str: str = "") -> datetime:
    return datetime.combine(today, time_today, today.tzinfo)


-def relative_time(hass: HomeAssistant, value: Any) -> Any:
+


Can you please remove this extra line#2098 this should fix your black failure.

Well, it doesn’t appear that this patch will be approved, so I haven’t bothered.

But I made the change anyway.

Thanks @rlippmann, I'm still following this issue and hoping it will be merged! Maybe @disforw can advise on next steps?

github-actions · 2023-10-24T11:05:04Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

haydndup · 2023-10-27T19:17:07Z

Who can approve this? Can't believe that such a simple change is literally taking months.

rlippmann · 2023-10-31T23:05:33Z

Who can approve this? Can't believe that such a simple change is literally taking months.

@haydndup Me neither. I've pretty much given up hope that it will every be merged. And I haven't rebased this in months.

haydndup · 2023-11-01T07:43:06Z

@disforw @epenet - can you please provide guidance here?

disforw · 2023-11-01T11:53:31Z

I got nothing. Epenet is the one you want..

emontnemery · 2023-11-22T21:11:27Z

homeassistant/helpers/template.py

@@ -2095,15 +2095,22 @@ def today_at(hass: HomeAssistant, time_str: str = "") -> datetime:
    return datetime.combine(today, time_today, today.tzinfo)


-def relative_time(hass: HomeAssistant, value: Any) -> Any:
+def relative_time(
+    hass: HomeAssistant, value: Any | datetime, is_future: bool = False, depth: int = 1


is_future is not a good name since the caller does not know if the passed time is in the future or in the past, can we come up with something better? Maybe name it remaining_time, time_until, elapsed_time or something like that?

allow_future?

No, that's not good.

We're not allowing or not allowing the timestamp to be in the future, we're either calculating time elapsed since some time in the past or we're calculating remaining time until some time in the future.

emontnemery · 2023-11-22T21:17:46Z

homeassistant/util/dt.py

@@ -267,36 +267,61 @@ def parse_time(time_str: str) -> dt.time | None:
        return None


-def get_age(date: dt.datetime) -> str:
+def get_age(date: dt.datetime, is_future: bool = False, depth: int = 1) -> str:


Given the name of the function, it seems very unnatural to add a parameter is_future to it; I would expect it to return a negative age for a time in the future.

Maybe split this up in multiple functions:

get_age, which returns the time elapsed since a time in the past

a new function, maybe named something like get_remaining_time which returns the remaining time to a time in the future

A helper functions which formats the time delta calculated by get_age and the new function as a string

What about:

Rename this to get_diff()

Change is_future to an enum, with values like DiffType.PAST_ONLY, DiffType.FUTURE_ONLY, DiffType.ANY -- this makes the code much easier to read than trying to figure out what a bool means in this context

Re-add get_age for backwards-compat that just calls get_diff(date, DiffType.PAST_ONLY, 1)

Note: There may not be value in having ANY, but being an enum at least opens this possibility. (I feel like I've needed a way to do a past-or-future date before, but I'm struggling to think of a good use case now)

Your suggestion is fine 👍

There may not be value in having ANY, but being an enum at least opens this possibility.

Let's not add functionality which we are not sure is needed.

It's been a while, but I think the is_future was meant to be more like an "allow future" flag.

An enum with 2 values is a boolean :)

Yes, but then please come up with a good name for a bool which clearly communicates how the behavior changes. I don't think allow_future is good because it's not clear setting it to True means a time in the past is no longer allowed.

Hence my proposal to leave the behavior of get_age unchanged and add a new function get_remaining_time which returns remaining time until some time in the future.

emontnemery · 2023-11-23T07:36:11Z

Please mark the PR as ready for review once the changes have been implemented.

…ant#105327) * Work-a-round orjson for `to_json` fiter in case dict key is str subclass * Add option instead * Remove json.dumps work-a-round * Update homeassistant/helpers/template.py * Fix test --------- Co-authored-by: Erik Montnemery <erik@montnemery.com>

…104948)

…t#105333)

…105310)

rlippmann · 2023-12-09T04:27:40Z

Oops, i made a little mess on the rebase.

emontnemery · 2023-12-09T08:27:27Z

how do i deprecate relative time? just raise a deprecation warning?

Yeah, I think you should do both:

Log a warning the first time the template function is called
Create an issue registry issue the first time the template function is called. The issue should explain that users need to update any function calling relative_time to instead call time_until and then restart home assistant to fix the issue

emontnemery · 2023-12-09T08:38:19Z

Is there any particular reason relative_time returns the original object if it isn't a datetime? And since we're creating new functions, do we want to keep it that way, or do something else?

This was standard practice in our old template functions, but we now prefer raising unless a default value is provided by the user. I think we should do that too here.

Question: how should we handle precision = 0? Exception? Same as precision = 1? Same as maximum precision?

I'd say just raise in that case since the function is used incorrectly?

Question 2: Do we assume a month is 30 days for the calculation? Or actually figure it out based upon the number of days in each month?

The existing code takes the easy route and assumes a month is 30 days. If we want to change that, we should do that in a separate PR, I think. There's a 3rd party library which supports calculations with calendar months: https://dateutil.readthedocs.io/en/stable/relativedelta.html

Question 3: If time_until is in the past, or time_since is in the future, do we raise a ValueError, or simply return 0 seconds?

I think the current behavior of relative_time is sane, i.e. return 0.

@MartinHjelmare Maybe you can comment too?

rlippmann · 2023-12-09T13:09:18Z

Is there any particular reason relative_time returns the original object if it isn't a datetime? And since we're creating new functions, do we want to keep it that way, or do something else?

This was standard practice in our old template functions, but we now prefer raising unless a default value is provided by the user. I think we should do that too here.

Sounds reasonable.

Question: how should we handle precision = 0? Exception? Same as precision = 1? Same as maximum precision?

I'd say just raise in that case since the function is used incorrectly?

Currently precision = 0 returns maximum precision. I don't think that's an unreasonable action, but I can change it to raise if that's better.

Question 2: Do we assume a month is 30 days for the calculation? Or actually figure it out based upon the number of days in each month?

The existing code takes the easy route and assumes a month is 30 days. If we want to change that, we should do that in a separate PR, I think. There's a 3rd party library which supports calculations with calendar months: https://dateutil.readthedocs.io/en/stable/relativedelta.html

That's what I was thinking too.

Question 3: If time_until is in the past, or time_since is in the future, do we raise a ValueError, or simply return 0 seconds?

I think the current behavior of relative_time is sane, i.e. return 0.

Actually, the current behavior returns the object. For time_until, I think returning 0 is a reasonable action since its use case would be for a countdown timer. I'm not sure if that's a good action for time_since.

@MartinHjelmare Maybe you can comment too?

rlippmann · 2023-12-09T13:31:35Z

how do i deprecate relative time? just raise a deprecation warning?

Yeah, I think you should do both:

Log a warning the first time the template function is called

Create an issue registry issue the first time the template function is called. The issue should explain that users need to update any function calling relative_time to instead call time_until and then restart home assistant to fix the issue

I'm not sure what you mean by this. Can you point me to a section in the code where this is already done?

emontnemery · 2023-12-14T08:01:22Z

how do i deprecate relative time? just raise a deprecation warning?

Yeah, I think you should do both:

Log a warning the first time the template function is called

Create an issue registry issue the first time the template function is called. The issue should explain that users need to update any function calling relative_time to instead call time_until and then restart home assistant to fix the issue

I'm not sure what you mean by this. Can you point me to a section in the code where this is already done?

Search the code base for async_create_issue, you will find many examples. There's also documentation here https://developers.home-assistant.io/docs/core/platform/repairs

Your warning function should do something like this:

from homeassistant.core import DOMAIN as HA_DOMAIN
import homeassistant.helpers.issue_registry as ir

def _warn_relative_time_deprecated(hass: HomeAssistant) -> None:
    issue_registry = ir.async_get_issue_registry(self.hass)
    issue_id = "template_function_relative_time_deprecated"
    if issue_registry.async_get_issue(HA_DOMAIN, issue_id):
        return
    ir.async_create_issue(
        hass,
        HA_DOMAIN,
        issue_id,
        breaks_in_ha_version="2024.6.0",
        is_fixable=False,
        severity=IssueSeverity.WARNING,
        translation_key=issue_id,
    )
    _LOGGER.warning("blablabla")

The translation for the issue should be added in homeassistant/components/homeassistant/strings.json

emontnemery · 2023-12-14T08:02:58Z

I'm setting this as draft, please mark as ready for review when the deprecation warning etc. are implemented.

emontnemery · 2023-12-14T08:10:55Z

Question: how should we handle precision = 0? Exception? Same as precision = 1? Same as maximum precision?

I'd say just raise in that case since the function is used incorrectly?

Currently precision = 0 returns maximum precision. I don't think that's an unreasonable action, but I can change it to raise if that's better.

OK, I agree that makes sense 👍

Question 3: If time_until is in the past, or time_since is in the future, do we raise a ValueError, or simply return 0 seconds?

I think the current behavior of relative_time is sane, i.e. return 0.

Actually, the current behavior returns the object. For time_until, I think returning 0 is a reasonable action since its use case would be for a countdown timer. I'm not sure if that's a good action for time_since.

I see. That behavior seems odd for time_since though. Maybe we should allow the user to specify the return value in case the passed in time is in the future for time_since and in the past for time_until?

emontnemery · 2023-12-14T08:11:45Z

Please update the PR title and PR description:

The PR title should be about the new template functions
The PR description should explain the new template functions and also mention relative_time has been deprecated

github-actions · 2024-02-12T16:06:26Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

anoblet · 2024-02-12T17:10:48Z

Bump

rlippmann requested review from epenet and a team as code owners January 18, 2023 03:42

home-assistant bot added cla-signed core new-feature labels Jan 18, 2023

rlippmann mentioned this pull request Jan 18, 2023

Deprecate relative_time in favor of time_since()/time_until() home-assistant/home-assistant.io#25631

Merged

6 tasks

MartinHjelmare changed the title ~~relative_time future and depth support~~ Add relative_time future and depth support Jan 18, 2023

emontnemery changed the title ~~Add relative_time future and depth support~~ Add future and depth support to the relative_time template function Jan 18, 2023

haydndup approved these changes Mar 6, 2023

View reviewed changes

disforw reviewed Jul 10, 2023

View reviewed changes

github-actions bot added the stale label Oct 24, 2023

github-actions bot removed the stale label Oct 27, 2023

emontnemery reviewed Nov 22, 2023

View reviewed changes

emontnemery marked this pull request as draft November 23, 2023 07:36

MartinHjelmare changed the title ~~deprecate relative_time in favor of time_since and time_until~~ Deprecate relative_time in favor of time_since and time_until Dec 8, 2023

jbouwh and others added 6 commits December 8, 2023 18:27

Add rollback on exception that needs rollback in SQL (home-assistant#…

a30b14a

…104948)

Bump plugwise to v0.34.5 (home-assistant#105330)

f3e98a9

Migrate homeassistant component tests to use freezegun (home-assistan…

88744b6

…t#105333)

Add myself as code owner for co2signal (home-assistant#105302)

11b0039

Add missing configuration for services.yaml in blink (home-assistant#…

cdd1587

…105310)

rlippmann requested review from gjohansson-ST, frenck and jpbede as code owners December 8, 2023 23:27

Merge branch 'dev' into relative_time

7717f26

rlippmann closed this Dec 9, 2023

rlippmann reopened this Dec 9, 2023

emontnemery marked this pull request as draft December 14, 2023 08:03

github-actions bot added the stale label Feb 12, 2024

github-actions bot removed the stale label Feb 12, 2024

MartinHjelmare added the stale label Feb 12, 2024

github-actions bot closed this Feb 19, 2024

github-actions bot locked and limited conversation to collaborators Feb 20, 2024

		@@ -2095,15 +2095,23 @@ def today_at(hass: HomeAssistant, time_str: str = "") -> datetime:
		return datetime.combine(today, time_today, today.tzinfo)


		def relative_time(hass: HomeAssistant, value: Any) -> Any:

Uh oh!

Deprecate relative_time in favor of time_since and time_until #86136

Deprecate relative_time in favor of time_since and time_until #86136

Uh oh!

Conversation

rlippmann commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

emontnemery commented Jan 18, 2023

Uh oh!

rlippmann commented Jan 18, 2023

Uh oh!

rlippmann commented Jan 25, 2023

Uh oh!

haydndup commented Feb 22, 2023

Uh oh!

rlippmann commented Mar 1, 2023

Uh oh!

rlippmann commented Mar 3, 2023

Uh oh!

haydndup commented Mar 6, 2023

Uh oh!

rlippmann commented Mar 6, 2023

Uh oh!

epenet commented Mar 6, 2023

Uh oh!

anoblet commented Apr 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rlippmann Jul 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 24, 2023

Uh oh!

haydndup commented Oct 27, 2023

Uh oh!

rlippmann commented Oct 31, 2023

Uh oh!

haydndup commented Nov 1, 2023

Uh oh!

disforw commented Nov 1, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rlippmann Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery commented Nov 23, 2023

Uh oh!

rlippmann commented Dec 9, 2023

Uh oh!

emontnemery commented Dec 9, 2023

Uh oh!

emontnemery commented Dec 9, 2023

Uh oh!

rlippmann commented Dec 9, 2023

Uh oh!

rlippmann commented Dec 9, 2023

Uh oh!

rlippmann commented Jan 18, 2023 •

edited

Loading

rlippmann Jul 10, 2023 •

edited

Loading

rlippmann Nov 23, 2023 •

edited

Loading

emontnemery commented Dec 14, 2023 •

edited

Loading