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

Months may be skipped in monthly metrics histories when last day of first month in series is greater than in a subsequent month #356

Open
bryanlandia opened this issue May 31, 2021 · 5 comments
Assignees

Comments

@bryanlandia
Copy link
Contributor

Appears that we never get any kind of metrics for February (unless probably it's the current month). This affects all types of metrics.

image

Looks like the problem is in metrics.previous_months_iterator

>>> from datetime import datetime
>>> date_for = datetime.utcnow().date()
>>> from figures import helpers, metrics
>>> date_for = helpers.as_date(date_for)
>>> date_for
datetime.date(2021, 5, 31)
>>> metrics.previous_months_iterator(month_for=date_for, months_back=6)
<generator object previous_months_iterator at 0x7fbc30ec0190>
>>> [mon for mon in metrics.previous_months_iterator(month_for=date_for, months_back=6)]
[(2020, 11, 30), (2020, 12, 31), (2021, 1, 31), (2021, 3, 31), (2021, 4, 30), (2021, 5, 31)]
@bryanlandia
Copy link
Contributor Author

I haven't yet verified if Feb's data is appearing as January or if February is just skipped altogether.

@bryanlandia bryanlandia changed the title February is skipped in monthly metrics histories Months may be skipped in monthly metrics histories when last day of first month in series is greater than in a subsequent month May 31, 2021
@bryanlandia
Copy link
Contributor Author

See note here https://dateutil.readthedocs.io/en/stable/rrule.html#classes

Per RFC section 3.3.10, recurrence instances falling on invalid dates and times are ignored rather than coerced:

Recurrence rules may generate recurrence instances with an invalid date (e.g., February 30) or nonexistent local time (e.g., 1:30 AM on a day where the local time is moved forward by an hour at 1:00 AM). Such recurrence instances MUST be ignored and MUST NOT be counted as part of the recurrence set.

This can lead to possibly surprising behavior when, for example, the start date occurs at the end of the month:

from dateutil.rrule import rrule, MONTHLY
from datetime import datetime
start_date = datetime(2014, 12, 31)
list(rrule(freq=MONTHLY, count=4, dtstart=start_date))
... # doctest: +NORMALIZE_WHITESPACE
[datetime.datetime(2014, 12, 31, 0, 0),
datetime.datetime(2015, 1, 31, 0, 0),
datetime.datetime(2015, 3, 31, 0, 0),
datetime.datetime(2015, 5, 31, 0, 0)]

bryanlandia added a commit that referenced this issue May 31, 2021
Avoid rrule when possibly iterating forward from days 29-31 of months,
rrule simply skip month if it tries to iterate to e.g., February 30.
bryanlandia added a commit that referenced this issue May 31, 2021
Avoid rrule when possibly iterating forward from days 29-31 of months,
rrule simply skip month if it tries to iterate to e.g., February 30.
@bryanlandia
Copy link
Contributor Author

See draft PR #358 for fix

@johnbaldwin
Copy link
Contributor

Thanks for this, @bryanlandia !

@bryanlandia
Copy link
Contributor Author

@johnbaldwin I think you probably saw this, but most likely the change in #360 could be removed once #358 is merged. Anyway, the problem onlly occurs in days 29, 30, and 31 of each month 🤣 so we have a little time to sort it out. (I need to write or update tests)

bryanlandia added a commit that referenced this issue Jun 4, 2021
Avoid rrule when possibly iterating forward from days 29-31 of months,
rrule simply skip month if it tries to iterate to e.g., February 30.

previous_months_iterator start num_months -1 back to incl. current

Since changing the method of providing a previous months generator, we
need to start one fewer months back or we lose the current month in
the output.  current_month should be last month returned in history
bryanlandia added a commit that referenced this issue Jun 8, 2021
Avoid rrule when possibly iterating forward from days 29-31 of months,
rrule simply skip month if it tries to iterate to e.g., February 30.

previous_months_iterator start num_months -1 back to incl. current

Since changing the method of providing a previous months generator, we
need to start one fewer months back or we lose the current month in
the output.  current_month should be last month returned in history
bryanlandia added a commit that referenced this issue Jun 10, 2021
Avoid rrule when possibly iterating forward from days 29-31 of months,
rrule simply skip month if it tries to iterate to e.g., February 30.

previous_months_iterator start num_months -1 back to incl. current

Since changing the method of providing a previous months generator, we
need to start one fewer months back or we lose the current month in
the output.  current_month should be last month returned in history
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