Skip to content

fix(#47): count step values from the field minimum - #48

Merged
kipe merged 1 commit into
kipe:mainfrom
Prafyl:fix/47-step-from-minimum
Sep 17, 2026
Merged

kipe merged 1 commit into
kipe:mainfrom
Prafyl:fix/47-step-from-minimum

Conversation

@Prafyl

@Prafyl Prafyl commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #47.

*/n was checking target % n == 0, which only works when the field starts at 0. Day of month and month start at 1, so */6 on day of month matched 6, 12, 18, 24, 30 instead of 1, 7, 13, 19, 25, 31, and */3 on month gave Mar, Jun, Sep, Dec instead of Jan, Apr, Jul, Oct.

_parse_arg now takes a min_value and checks (target - min_value) % n == 0. is_now passes min_value=1 for the dom and month fields. Minute, hour and day of week start at 0, so they keep the default and are unchanged.

Two existing assertions had to change. Like you said in the issue, test_dom.py and test_month.py both use June 18th, and 18 is divisible by 6 and June is divisible by 2, so they were passing on the old behaviour:

  • test_dom.py: * * */6 * * is now False on the 18th
  • test_month.py: * * * */2 * is now False in June, * * * */5 * is now True

Added test_dom_step and test_month_step, which use dates on and off the step and check that */n gives the same answer as the equivalent 1-31/n and 1-12/n.

Coverage stays at 100%, and flake8, pylint and mypy are clean.

*/n was checking target % n == 0, which only works when the field starts
at 0. Day of month and month start at 1, so */6 on day of month matched
6, 12, 18, 24, 30 instead of 1, 7, 13, 19, 25, 31, and */3 on month gave
Mar, Jun, Sep, Dec instead of Jan, Apr, Jul, Oct.

_parse_arg now takes a min_value and checks (target - min_value) % n == 0.
is_now passes min_value=1 for the dom and month fields. Minute, hour and
day of week start at 0, so they keep the default and are unchanged.

test_dom.py and test_month.py both use June 18th, and 18 is divisible by 6
and June is divisible by 2, so two of their assertions were passing on the
old behaviour and had to be corrected. Added test_dom_step and
test_month_step to cover dates on and off the step.
@Prafyl
Prafyl force-pushed the fix/47-step-from-minimum branch from 821f4b9 to 16813d8 Compare September 16, 2026 11:42
@kipe

kipe commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Thanks, looks good 👍

@kipe
kipe merged commit 7c40c1b into kipe:main Sep 17, 2026
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

Successfully merging this pull request may close these issues.

*/n on day of month and month counts from 0, so it matches the wrong days

2 participants