Roundup Tracker - Issues

Issue 2550858

classification
Month intervals at the end of long months yield the wrong day
Type: behavior Severity: normal
Components: Infrastructure Versions: devel, 1.5, 1.4, 1.3
process
Status: new
: Calender 'next month' link fails on day 31 of a month
View: 2551208
: : ThomasAH, ber, rouilj, schlatterbeck
Priority: normal : patch

Created on 2014-10-31 15:40 by ThomasAH, last changed 2022-06-20 11:12 by schlatterbeck.

Files
File name Uploaded Description Edit Remove
fix-popup-cal-month.patch ThomasAH, 2022-06-01 09:39
fix-popup-cal-year.patch ThomasAH, 2022-06-01 09:41
Messages
msg5156 Author: [hidden] (ThomasAH) Date: 2014-10-31 15:40
Tested in the current devel version, but I assume it affects all
previous versions, too:

>>> from roundup import date
>>> date.Date("2014-10-31 +1m")
<Date 2014-12-01.00:00:00.000>
This should yield 2014-11-30

>>> date.Date("2014-10-31 -1m")
<Date 2014-10-01.00:00:00.000>
This should yield 2014-09-30

This affects at least the popup calender and searching for date ranges.

The culprit is the following code from roundup.date.Date addInterval():

    while month < 1 or month > 12 or day < 1 or day > get_mdays(year,month):
        # now to day under/over
        if day < 1:
            # When going backwards, decrement month, then increment days
            month -= 1
            day += get_mdays(year,month)
        elif day > get_mdays(year,month):
            # When going forwards, decrement days, then increment month
            day -= get_mdays(year,month)
            month += 1

        # possibly fix up the month so we're within range
        while month < 1 or month > 12:
            if month < 1: year -= 1; month += 12 ; day += 31
            if month > 12: year += 1; month -= 12

Reproducible by running "./run_tests.py test_dates.py" after applying the
following patch:

diff -r b76710818d31 test/test_dates.py
--- a/test/test_dates.py	Mon Oct 20 14:10:32 2014 -0400
+++ b/test/test_dates.py	Fri Oct 31 16:35:41 2014 +0100
@@ -122,6 +122,19 @@
         date = Date('2000-01-01 + 2m')
         ae(str(date), '2000-03-01.00:00:00')
 
+        date = Date('2000-01-29 + 1m')
+        ae(str(date), '2000-02-29.00:00:00')
+        date = Date('2001-01-29 + 1m')
+        ae(str(date), '2000-02-28.00:00:00')
+        date = Date('2001-01-30 + 1m')
+        ae(str(date), '2000-02-28.00:00:00')
+        date = Date('2000-10-31 + 1m')
+        ae(str(date), '2000-11-30.00:00:00')
+        date = Date('2000-10-31 - 1m')
+        ae(str(date), '2000-09-30.00:00:00')
+        date = Date('2000-03-31 - 1m')
+        ae(str(date), '2000-02-29.00:00:00')
+
         date = Date('2000-01-01') + Interval('60d')
         ae(str(date), '2000-03-01.00:00:00')
         date = Date('2001-01-01') + Interval('60d')
msg6818 Author: [hidden] (ThomasAH) Date: 2019-11-13 15:39
A similar problem occurs when adding/substracting years when the
current day does not exist in the target year, i.e. only for
February 29th:

This works as expected:
>>> date.Date('2020-02-29') + date.Interval('+4y')
<Date 2024-02-29.00:00:00.000>

Here you can see the issue:
>>> date.Date('2020-02-29') + date.Interval('+1y')
<Date 2021-03-01.00:00:00.000>
>>> date.Date('2020-02-29') + date.Interval('-1y')
<Date 2019-03-01.00:00:00.000>
msg7553 Author: [hidden] (ThomasAH) Date: 2022-06-01 09:39
This patch is from 2019, not sure if it still applies or works correctly:


fix-popup-cal-month.patch:

Calendar Popup: Fix prev/next month for shorter months

Jump to correct month if the prev/next month has fewer days (e.g. 28/29/30)
than the currently selected day (e.g. 29/30/31)

Based on a fix by Ludwig Reiter <ludwig.reiter@intevation.de>
msg7554 Author: [hidden] (ThomasAH) Date: 2022-06-01 09:41
Second patch from 2019:

fix-popup-cal-year.patch

Calendar Popup: Fix prev/next year for shorter February

Patch by Ludwig Reiter <ludwig.reiter@intevation.de>
msg7555 Author: [hidden] (schlatterbeck) Date: 2022-06-01 09:52
Thanks, Thomas, for reminding us of the existence of this old issue.

Concerning the patches, as outlined in issue2551208: I don't think this should be fixed in the calendar popup code but instead in roundup.date (after adding tests that specify what results should look like).

The bug has the potential of turning up in other places where the Data + Interval logic doesn't do the right thing.
msg7556 Author: [hidden] (ThomasAH) Date: 2022-06-01 10:08
> but instead in roundup.date

I agree that this would be better.

> specify what results should look like

This is the hard part :)
msg7580 Author: [hidden] (rouilj) Date: 2022-06-16 23:17
It looks like the current code acts the same as gnu date:

1 month adds the number of days in month of starting date
take that date as the new starting date and add another current month's
worth of days.

so 2022-01-29 + 1 month adds 31 days -> 2022-03-01,
   2022-01-29 + 2 months adds 31 + 28 days (non-leap year) -> 2022-03-29

   2020-02-10 + 1 month (leap year) + 29 days

now when going backwards it subtracts the number of days in the previous month:

   2022-05-29 - 1 month subtracts 30 days (April) -> 2022-04-29

Similarly:

   2022-01-10 - 1 month subtracts 31 days (dec) -> 2021-12-10

So I claim roundup is doing a right thing here at least.

Thomas I think your desired test cases are handled by changing to:

  adding the number of days in the next month
or
  subtracting the days in the current month?

Is that less confusing/more desirable or more correct according to some authority?
msg7581 Author: [hidden] (ThomasAH) Date: 2022-06-17 05:12
* John Rouillard <issues@roundup-tracker.org> [20220617 01:37]:
> Thomas I think your desired test cases are handled by changing to:
> 
>   adding the number of days in the next month

No, this doesn't work in months with more days than the next, e.g.:
2022-05-01 + 1 month (30 days) = 2022-05-31

-> still the same month instead of the next

> or
>   subtracting the days in the current month?

This would work.

So either changing the UI part instead of the date calculations is
the right thing here or we have to accept that
date + 1 month - 1 month != date
and just properly document it.

And don't forget that +/- 1 year in February needs to be adjusted,
too:
  2024-02-29 + 1 year = 2025-02-28
  2025-02-28 - 1 year = 2024-02-28 (or -29?)
msg7583 Author: [hidden] (schlatterbeck) Date: 2022-06-20 11:12
On Thu, Jun 16, 2022 at 11:17:54PM +0000, John Rouillard wrote:
> 
> John Rouillard added the comment:
> 
> It looks like the current code acts the same as gnu date:

John, the mechanism is mainly used for the prev/next links in the
calendar widget. So if this sometimes does not navigate to the
previous/next month and sometimes jumps two months this is a bug. No
matter what Gnu date says :-)

I think we should fix it so that it works for the documented bug cases.

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
History
Date User Action Args
2022-06-20 11:12:36schlatterbecksetmessages: + msg7583
2022-06-17 05:12:51ThomasAHsetmessages: + msg7581
2022-06-16 23:19:49rouiljsetsuperseder: Calender 'next month' link fails on day 31 of a month
2022-06-16 23:17:54rouiljsetmessages: + msg7580
2022-06-01 10:08:01ThomasAHsetmessages: + msg7556
2022-06-01 09:52:45schlatterbecksetmessages: + msg7555
2022-06-01 09:41:12ThomasAHsetfiles: + fix-popup-cal-year.patch
messages: + msg7554
2022-06-01 09:39:57ThomasAHsetfiles: + fix-popup-cal-month.patch
nosy: + schlatterbeck, rouilj
messages: + msg7553
keywords: + patch
2019-11-13 15:39:56ThomasAHsetmessages: + msg6818
2014-12-11 09:00:39bersetnosy: + ber
2014-10-31 15:40:52ThomasAHcreate