Skip to content
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

Possible bug or documentation bug in TimeRecurrence() get_next() and get_prev() methods #224

Open
mo-jareddrayton opened this issue Jan 9, 2023 · 2 comments
Milestone

Comments

@mo-jareddrayton
Copy link

TimeRecurrence() get_next() and get_prev() Behaviour

The docstrings for the get_next() and get_prev() state the following.

"Return the next timepoint after this timepoint in the recurrence series, or None."
"Return the previous timepoint before this timepoint in the recurrence series, or None."

However, this only seems to work properly when providing a TimePoint that itself is a valid time in the recurrence series.

If you pass a TimePoint that isn't a valid TimePoint within the series, the methods will return the given TimePoint with the cycle duration added/subtracted, rather than returning None which I think should be the expected behaviour based on the docstrings.

import metomi.isodatetime.parsers as parse

recurrence = parse.TimeRecurrenceParser().parse('R/2000/P4Y')
# Example where the given TimePoint does exist within the series.
date_time = parse.TimePointParser().parse('2000-01-01T00:00Z')
print(date_time)
print(recurrence.get_next(date_time))
print("Is this a valid TimePoint in this recurrence series?")
print(recurrence.get_is_valid(recurrence.get_next(date_time)))
2000-01-01T00:00:00Z
2004-01-01T00:00:00Z
Is this a valid TimePoint in this recurrence series?
True
# Example where the given TimePoint does not exist in the series.
date_time = parse.TimePointParser().parse('2001-01-01T00:00Z')
print(date_time)
print(recurrence.get_next(date_time))
print("Is this a valid TimePoint in this recurrence series?")
print(recurrence.get_is_valid(recurrence.get_next(date_time)))
2001-01-01T00:00:00Z
2005-01-01T00:00:00Z
Is this a valid TimePoint in this recurrence series?
False
# Example where the given TimePoint does not exist in the series.
date_time = parse.TimePointParser().parse('2007-01-01T00:00Z')
print(date_time)
print(recurrence.get_prev(date_time))
print("Is this a valid TimePoint in this recurrence series?")
print(recurrence.get_is_valid(recurrence.get_prev(date_time)))
2007-01-01T00:00:00Z
2003-01-01T00:00:00Z
Is this a valid TimePoint in this recurrence series?
False

I think changing self.get_is_in_bounds() to self.get_is_valid in

def get_is_valid(self, timepoint: "TimePoint") -> bool:
may be enough to fix this?

    def get_next(self, timepoint: "TimePoint") -> "TimePoint":
        """Return the next timepoint after this timepoint in the recurrence
        series, or None."""
        if self._repetitions == 1 or timepoint is None:
            return None
        next_timepoint = timepoint + self._duration
        if self._get_is_in_bounds(next_timepoint):
            return next_timepoint
        return None

    def get_prev(self, timepoint: "TimePoint") -> "TimePoint":
        """Return the previous timepoint before this timepoint in the
        recurrence series, or None."""
        if self._repetitions == 1 or timepoint is None:
            return None
        prev_timepoint = timepoint - self._duration
        if self._get_is_in_bounds(prev_timepoint):
            return prev_timepoint
        return None
@MetRonnie
Copy link
Contributor

Hi Jared, does TimeRecurrence().get_first_after() do what you are after?

I admit the get_next()/get_prev() docstrings may be misleading, and there is no get_first_before() method either

@MetRonnie MetRonnie added this to the 3.0.1 milestone Jan 10, 2023
@mo-jareddrayton
Copy link
Author

Hi Ronnie, yes thanks I've already been using TimeRecurrence().get_first_after() and it works very nicely. A get_first_before() would be a great addition too!

@MetRonnie MetRonnie modified the milestones: 3.0.1, 3.1.0, 3.2.0 Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants