Skip to content

Commit

Permalink
Calculate all pivot points based on end pivot
Browse files Browse the repository at this point in the history
Previously, monthly pivots calculated from the end of a month failed to
account for the variation in the number of days per month. This was due
to subtle differences between subtracting an isodate.Duration against a
decremental date (previous implementation) and subtracting an
isodate.Duration with a multiplier against a fixed date (new
implementation).

Tests updated with the new behavior.
  • Loading branch information
victorlin committed Mar 7, 2023
1 parent b22a308 commit dde579b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion augur/frequency_estimators.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def get_pivots(observations, pivot_interval, start_date=None, end_date=None, piv
pivot = end
while pivot >= start:
pivots.appendleft(pivot)
pivot = pivot - delta
pivot = end - delta * len(pivots)

pivots = np.array([numeric_date(pivot) for pivot in pivots])

Expand Down
10 changes: 8 additions & 2 deletions tests/test_frequencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,23 @@ def test_get_pivots_by_invalid_unit():
(
"2022-01-31",
"2022-03-31",
("2022-02-28", "2022-03-31")
("2022-01-31", "2022-02-28", "2022-03-31")
),
# Note that Jan 31 to Apr 30 gives the same amount of pivot points as
# Jan 31 to Mar 31.
(
"2022-01-31",
"2022-04-30",
("2022-02-28", "2022-03-30", "2022-04-30")
),
# However, in practice, the interval is more likely to be Jan 30 to Apr
# 30 as long as the start date is calculated relative to the end date
# (i.e. start date = 3 months before Apr 30 = Jan 30).
# That interval includes an additional pivot point as expected.
(
"2022-01-30",
"2022-04-30",
("2022-02-28", "2022-03-30", "2022-04-30")
("2022-01-30", "2022-02-28", "2022-03-30", "2022-04-30")
),
]
)
Expand Down

0 comments on commit dde579b

Please sign in to comment.