Eliminate error calls in report periods #2457
Conversation
Xitian9
force-pushed
the
splitSpan
branch
2 times, most recently
from
169fdf7 to
97a0494
Compare
|
I'll hold this one for now as I don't have an open bug that needs it. If you have time, #2454 would be a great one to look at next for 1.50.1. |
|
Hola @Xitian9.. This PR has two commits, so I don't understand git log's showing me just one when I check out this branch:
could you rebase when you have a chance. |
|
Rebased. |
| _ -> d | ||
|
|
||
|
|
||
| intToDay = ModifiedJulianDay . toInteger |
There was a problem hiding this comment.
The Modified Julian Day is a standard count of days, with zero being the day 1858-11-17.
I think this is from an earlier PR, but it sounds like it might not fit with our support of all years ? (CE currently, maybe BCE some day)
There was a problem hiding this comment.
Yes, this is from an earlier commit, and yes it technically does not support all years.
- On a 64-bit system this will support all years between 252 quadrillion BC (well before the development of currency) and 252 quadrillion AD (at which point we'll be more worried about powering our computers since all stars will have become black holes or white dwarfs).
- On a 32-bit system, it will support all years between 5 877 751 BC and 5 881 468 AD.
We could extend support to all integers. This would incur a small performance penalty as we'd have to switch from IntMap to Map internally. I personally think the supported date ranges are fine, but happy for you to make a call.
There was a problem hiding this comment.
Oh wow. So 0 to 1858 is not a problem then. Great.
There was a problem hiding this comment.
AI gave this overview of the latest date types:
Date-related Types Overview
Basic date types (Types.hs:103-123):
- EFDay - exact or flexible date (Exact Day | Flex Day)
- DateSpan - optional start/end dates (DateSpan (Maybe EFDay) (Maybe EFDay))
Report period types (Types.hs:129-158):
- Period - high-level periods (DayPeriod, MonthPeriod, etc.)
- Interval - report intervals (Days, Weeks, Months, Quarters, Years, etc.)
Period data structures:
- PeriodData a (Types.hs:756) - data for report periods + pre-period
- pdpre :: a - historical/pre-report data
- pdperiods :: IntMap a - data per period (Day as IntMap key)
- DayPartition (DayPartition.hs:37) - well-formed time partition
- Newtype wrapping PeriodData Day
- Opaque constructor enforces contiguous spans
- Invariants: non-empty, contiguous, end = start-1 of next
Key functions:
- splitSpan :: Interval -> DateSpan -> Maybe DayPartition - split span into
periods - lookupDayPartition :: Day -> DayPartition -> (Maybe Day, Day) - find period
containing day - dayPartitionToDateSpans :: DayPartition -> [DateSpan] - convert to list of
spans
Does it sound about right to you ?
DayPartition is PeriodData with mandatory smart constructors to guarantee invariants. Is it valuable to still have the separate PeriodData type ?
If so we should probably expand the description of both types, making things more approachable.
I have installed this locally and will try to exercise it as well.
|
Now also available as https://github.com/simonmichael/hledger/releases/tag/nightly binaries. |
|
Yes, I think that description is basically accurate, though I think only We would still need the separate |
This allows us to guarantee that the report periods are well-formed and don't contain errors (e.g. empty spans, spans not contiguous, spans not a partition). Note the underlying representation is now for disjoint spans, whereas previously the end date of a span was equal to the start date of the next span, and then was adjusted backwards one day when needed.
This eliminates all error calls from the chain calculating report periods.
This eliminates the last few error calls that can occur in calculating the report spans of a multi-balance report.
DayPartitionforPeriodData Day. By not exporting the constructor, we can ensure that allDayPartitions are well-formed.splitSpanis moved intoHledger.Data.DayPartitionand now producesMaybe DayPartitiondirectly, rather than passing through[DateSpan].That said, this introduced a fair amount of code churn in order to eliminate the last few error calls. If you feel that the benefit is not worth the cost, let me know.