hledger icon indicating copy to clipboard operation
hledger copied to clipboard

Inconsistency in period expressions interpretations

Open ony opened this issue 7 years ago • 13 comments

I've noticed weird inconsistency in treating period expressions:

  • every 1st day of month from 2017/1/5: starts from 2017/2/1 (valid)
  • monthly from 2017/1/5: starts from 2017/1/1 (outside of the range)

Though should be easy to fix.

ony avatar Nov 21 '17 21:11 ony

If we want to make monthly to be day-agnostic. Then we probably should have behaviour like:

monthly from 2017/1/5: gives dates 2017/1/5, 2017/2/1, 2017/3/1 ....

ony avatar Nov 21 '17 21:11 ony

FYI it's probably using the same code that multi-period reports do - extending the first & last periods to the natural period boundaries, so that numbers in the first/last columns are comparable with the inner columns.

ghost avatar Nov 21 '17 21:11 ghost

I think i fixed this in runPeriodicTransactioms in #645

adept avatar Nov 21 '17 22:11 adept

Posted too soon, before reading your comments there :(

adept avatar Nov 21 '17 22:11 adept

@hledgerlist , you are right I'm refering to splitSpan used in multi-period reports. Ideally we should specify monthly from 2017/1. I see that it makes rendering of column headers much better than having precise boundaries of first and last period. It just feels a bit unnatural for this function. If it only made like this for report we probably should truncate days/month for rendering.

I would expect all next commands be equivalent:

  • hledger bal -p 'monthly from 2017/11/15'
  • hledger bal -p 'monthly' -b 2017/11/15
  • hledger bal -M -b 2017/11/15
  • hledger bal -M date:'from 2017/11/15'

And -b as well as date (part of query) to be honored precisely. But it is too late for such decision, I guess. So we can close this ticket (that's why I dropped "bug" label).

ony avatar Nov 21 '17 22:11 ony

And I gues current behavior is definitely more useful in terms of "I want to see week report that includes that days" (hledger bal -p 'weekly in today').

ony avatar Nov 21 '17 22:11 ony

I didn't found in documentation any mention about extending ranges to include complete periods. The only place is code documentation for splitSpan:

-- | Split a DateSpan into one or more consecutive whole spans of the specified length which enclose it.
-- If no interval is specified, the original span is returned.

It might be not clear from a frist glance, but I believe it states that it produce consecutive spans of specified length that together enclose provided DateSpan. This means that behaviour of every 1st day of month from 2017/1/5 should actually include date 2017/1/5 in one of the produced spans like monthly from 2017/1/5 do.

This behaviour becomes consistent with change 4049455f2 which is part of #654 .

ony avatar Nov 26 '17 01:11 ony

Agreed, this should be mentioned in docs.

Re "monthly from 2017/1/5" - I thought after #654 maybe this would give an error (interval is monthly but does not start on a month boundary) but it didn't. I guess only the "every ..." form checks that.

If we did want to support intervals not starting on the natural boundary (and I don't know how big a task that is), I guess we'd expect "monthly from 2017/1/5" to generate 2017/1/5, 2017/2/5, 2017/3/5 etc. ?

That gets into what to do about "monthly from 2017/1/30" etc. and all of the date-adjusting options Carel mentioned. If we go further down this route we should probably consider closely following the semantics (at least) of ISO 8601 and avoid reinventing everything.

simonmichael avatar Nov 27 '17 00:11 simonmichael

After 654 all of "weekly", "every week", "monthly","every month", ... (same for quarter and year) give errors, but only when used as period expressions in transactions. I opted not to put this behavior in parsePeriodExpr parser itself to preserve the current behavior of command-line parsing. Rationale: one can imagine a situation where report is ran with -b , -e and -M, which results in the same perdion expression as "monthly from to ", where dates are not lined up on month boundaries, and this is exactly what user wants/needs for whatever reason.

If one wants 2017/1/5, 2017/2/5, 2017/3/5, one can use -p "everth 5th day [of month]" for that.

In my opinion there are several dimensions with questions here::

  1. (internals dimension) Right now internally there two kinds of monthly intervals - one that always starts on month boundary (Month Int) and one that start on arbitrary day (DayOfMonth Int) -- should they be unified or not?
  2. (journal syntax dimension) Period expressions are used in periodic transactions (only). "monthly from 2017/1/5" generates errors. Is this ok? Is this enough? Should it be made to work as "every 5th day of month" automagically? What would be more/less confusing for users?
  3. (cli dimension). Right now there are two intersecting ways to specify "period expression": one way is to use -p only (-p "monthly from 2017/1/5"), another one is to use a combination of -p/-b/-e/-M/-Q/-Y. Potential questions here: is it ok to use the same parser for -p and for journal syntax? Should that parser (parsePeriodExpr) be aware of the context in which parsing is done (for journal or for cli) and perform different checks depending on context, or is it better to enforce uniformity here? What about the case when parser never sees whole input at once (when user writes -b date -e date -p monthly instead of -p "monthly from date to date"), do we enforce consistent behavior? Is it ok from UX point of view to have a situation where you just saw the report with some values for -b/-e, but when you add -Y to your commandline, things suddenly fail?

Note that as long as -b -e -M -Q -Y -p exist (and they probably should keep existing to simplify switch-over from ledger), you effectively have special syntax for constructing period expression with implicit unspoken rules for how things combine. Even if in-journal period expressions are forced to be ISO8601-only, all the open questions for cli would remain unresolved.

On Mon, Nov 27, 2017 at 12:40 AM, Simon Michael [email protected] wrote:

Agreed, this should be mentioned in docs.

Re "monthly from 2017/1/5" - I thought after #654 https://github.com/simonmichael/hledger/pull/654 maybe this would give an error (interval is monthly but does not start on a month boundary) but it didn't. I guess only the "every ..." form checks that.

If we did want to support intervals not starting on the natural boundary (and I don't know how big a task that is), I guess we'd expect "monthly from 2017/1/5" to generate 2017/1/5, 2017/2/5, 2017/3/5 etc. ?

That gets into what to do about "monthly from 2017/1/30" etc. and all of the date-adjusting options Carel mentioned. If we go further down this route we should probably consider closely following the semantics (at least) of ISO 8601 and avoid reinventing everything.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/simonmichael/hledger/issues/650#issuecomment-347052401, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHNKukf0x2K3I4H_2cNFMfRd96HhrGoks5s6gTqgaJpZM4QmfHh .

-- D. Astapov

adept avatar Nov 27 '17 14:11 adept

Related, @eamsden says currently "biweekly" must start on Sundays.

Period expressions are in need of new documentation and systematic review.

simonmichael avatar Nov 27 '17 15:11 simonmichael

Second sticking to ISO8601 standard, existing date does, eg (interaction with shell):

[hledger]% date --date='2017/08/05 + 1 day'
Sun Aug  6 00:00:00 CDT 2017
[hledger]% date --date='2017/08/05 + 8 day'
Sun Aug 13 00:00:00 CDT 2017
[hledger]% date --date='2017/08/05 + 1 week'
Sat Aug 12 00:00:00 CDT 2017
[hledger]% date --date='2017/08/05 + 1 month'
Tue Sep  5 00:00:00 CDT 2017
[hledger]% date --date='2017/08/05 + 1 month + 1 day'
Wed Sep  6 00:00:00 CDT 2017
[hledger]% date --date='2017/01/30 + 1 month'        
Thu Mar  2 00:00:00 CST 2017

ehildenb avatar Sep 03 '18 04:09 ehildenb

@ehildenb, interesting, though are you sure date sticks to ISO8601 ? It seems to me it overlaps it, that's all. I found docs for the above at https://www.gnu.org/software/coreutils/manual/html_node/Relative-items-in-date-strings.html.

@adept's questions are a good start at a more comprehensive explanation of our semantics. My guess is we'll want to

  • make period expressions in queries and in periodic transactions more consistent/the same
  • not reject expressions just because they don't start on a tidy interval boundary
  • develop the docs, and the syntax if needed, to make all behaviour clearer, more consistent and more expressive
  • make sure we offer sufficient debug output for period expression troubleshooting
  • maybe experiment with an optional alternative syntax such as ISO8601, after the above is under control

simonmichael avatar Sep 03 '18 17:09 simonmichael

Related, here is Robert Nielsen's review of current functionality, from the mail list:

Following up on this, I compared hledger’s options for scheduling transactions with two commercial programs that schedule events: MS Outlook and Calendar (for the Mac). Why compare hledger to Outlook and Calendar, when the latter two are not specific to finances? For one, they are used for scheduling recurring events, and that is the function we are looking at in hledger, even if the hledger “event” is a financial transaction. In addition, Outlook and Calendar are widely used, and presumably a lot of thought has gone into their design. Finally, I have access to these programs and can test what they do and don’t do. In other words, it’s one thing to read about a program; it is another to be able to do hands on testing.

Also, it’s very possible that I have missed some ways one or more of the programs works, so corrections are welcomed!

I have organized the comparison starting with a summary, and then examining in more detail five basic options for scheduling: daily, weekly, monthly, quarterly and yearly.

Summary There is considerable overlap between hledger and the two commercial programs. However, there are some differences.

Some of the things that hledger doesn’t have are:

• A distinction between weekend days and the rest of the workweek (note, depending on the locale, one country’s weekend day may be another country’s workday) • An option for “last” as in the last Monday of a month

On the other hand, hledger has some things that the commercial programs don’t:

• An explicit way to schedule quarterly events • An option to extend 1st, 2nd, 3rd, 4th, and 5th indefinitely (there must be an upper limit, but I didn’t come across one in my testing)

And there are even different definitions between hledger and the commercial programs for what might appear at first glance to be the same thing. For example, all programs have the concept of 1st, 2nd, 3rd, and 4th (as in 4th Monday). However, there are differences in the definition.

Daily and Weekly First, all three programs seem to handle daily and weekly scheduling largely the same. Daily events can be scheduled every X number of days. Weekly events can be scheduled on a specific day of the week (e.g., every Monday). Events can also be scheduled every Xth day of the week, such as every second Tuesday. That is, the event would take place every other week on a Tuesday.

Monthly In terms of similarities, the 3 programs allow for the option to schedule

• Every X month(s) • Every 1st, 2nd, 3rd, and 4th Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, and Saturday • A date, such as the 23rd of a month

As to differences, the two commercial programs allow the following, which the current version of hledger doesn’t have:

• An option for “last,” as in the last Tuesday of the month • Options to select “weekday” or “weekend day” as in the 3rd weekday of the month or the last weekday of the month

Note that hledger does have “weekday” option; however, “weekday” in the commercial programs means the part of the week traditional or normative for working (Monday to Friday in many places, but this depends on the locale—see https://en.wikipedia.org/wiki/Workweek_and_weekend for more information on this topic). Note, though, that in hledger, weekday means any of the seven days of the week.

Both hledger and Calendar offer an option for the 5th Monday, Tuesday, Wednesday, etc. of the month (e.g., “5th Monday of July”). However, with Calendar, if you set up something for the 5th Monday of every month, Calendar will schedule something only if there is a 5th Monday. In contrast, hledger will schedule something every 5th Monday, no matter where it falls in a month.

In addition, hledger differs from the other two programs in that it allows for 6th, 7th, 8th, etc. Monday, Tuesday, Wednesday, etc. of the month. While at first glance this sounds impossible (i.e., there is no “7th Monday of a month”), hledger calculates the nth Monday, Tuesday, etc. beginning from whatever period is specified or understood. For example, the 11th Monday of 2018 is March 12, 2018. Similarly, the 11th Monday of January (of 2018) is exactly the same date (March 12, 2018) as hledger counts 11 Mondays beginning with the first Monday in January until it arrives at March 12.

Quarterly While hledger supports quarterly intervals, neither Outlook nor Calendar explicitly have this option (though it would be possible to define something that repeated quarterly).

Yearly All three programs can work with yearly intervals, but Calendar adds the option to specify every N years.

When specifying a yearly interval, Calendar and Outlook require a month and a day to be specified. (Calendar additionally gives the option to specify multiple months, so you can have something yearly that happens in specific months, for example: January, February, and September.) Along with the month, a specific day is required, the same as with a monthly interval, which I will summarize for the two commercial programs:

• Every 1st, 2nd, 3rd, and 4th Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, and Saturday • An option for “last,” as in the last Tuesday of the month • Options to select “weekday” or “weekend day” as in the 3rd weekday of the month or the last weekday of the month • A date, such as the 23rd of a month

When specifying a yearly interval for hledger, you also specify a month and a day (which are implied if no month or day is stated). As stated above in the discussion for monthly intervals, hledger has the following options for days:

• Every 1st, 2nd, 3rd, etc. Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, and Saturday • A date, such as the 23rd of a month

simonmichael avatar Dec 31 '18 09:12 simonmichael