classification
Title: Getting the starting date of iso week from a week number and a year.
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder: strptime should implement %G, %V and %u directives
View: 12006
Assigned To: Nosy List: Erik Cederstrand, Esben.Agerbæk.Black, belopolsky, lemburg, mark.dickinson, pitrou
Priority: normal Keywords: needs review, patch

Created on 2012-03-27 12:01 by Esben.Agerbæk.Black, last changed 2015-10-02 22:00 by belopolsky. This issue is now closed.

Files
File name Uploaded Description Edit
isodates.patch Esben.Agerbæk.Black, 2012-03-27 18:50 Patch to get a date object with from_iso_week(year, week, day=1) review
isodates.patch Esben.Agerbæk.Black, 2012-03-28 17:20 More sane patch to get a date object with from_iso_week(year, week, day=1) review
isodates.patch Esben.Agerbæk.Black, 2012-03-28 18:39 Patch now includes enhancement, tests and documentation review
isodates.patch Esben.Agerbæk.Black, 2012-03-31 10:11 Patch made more consistent and minor error in tests fixed review
isodates.patch Esben.Agerbæk.Black, 2012-04-08 11:38 Corrections review
isodates.patch Esben.Agerbæk.Black, 2012-04-22 21:12 Fixes a previous and buggy patch. review
Messages (24)
msg156913 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-03-27 12:01
An enhancement request to make available the date of the first iso weekday of any week of any year.
msg156951 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-03-27 18:50
I have made a patch as per the "Lifecycle of a patch" quick guide.
(http://docs.python.org/devguide/patch.html#lifecycle-of-a-patch)
msg156991 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-03-28 17:20
Patch updated with sanity checks.
msg156997 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-03-28 18:39
Patch updated with tests and documentation
msg157905 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-09 21:29
Some comments:

- you also need to modify the C version in Modules/_datetimemodule.c
(make sure the tests exercise both versions!)

- the doc needs a "versionadded" tag

- I don't understand this:

+        if self.isocalendar()[1] != 1:
+            if self.weekday() > 3:  # Jan 1 is not in week one
+                self += timedelta(7 - self.weekday())

What if self.weekday() <= 3 ? Surely some adjustment is needed as well?
(you should probably add more tests to cover the various cases)

- this comment:

+        # Test that we get the OverflowError when the year has 52 weeks

seems wrong since ValueError is raised
msg157907 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-04-09 21:50
Before you invest in a C version, let's discuss whether this feature is desirable.  The proposed function implements a very simple and not very common calculation.  Note that even dateutil does not provide direct support for this: you are instructed to use relativedelta to add weeks to January 1st of the given year.

If we are going to add yet another way to construct dates, I would like to consider some universal solution.  For example, a "make_date" function that would behave similar to the timedelta constructor: take a large number of keyword arguments and return a date.  For example,

make_date(year=2000, isoweek=5, weekday=3)
make_date(year=2000, isoday=63)
etc.
msg157912 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-09 22:20
Alexander Belopolsky wrote:
> 
> Alexander Belopolsky <alexander.belopolsky@gmail.com> added the comment:
> 
> Before you invest in a C version, let's discuss whether this feature is desirable.  The proposed function implements a very simple and not very common calculation.  Note that even dateutil does not provide direct support for this: you are instructed to use relativedelta to add weeks to January 1st of the given year.

Which is wrong, since the start of the first ISO week of a year
can in fact start in the preceeding year...

http://en.wikipedia.org/wiki/ISO_week_date

and it's not a simple calculation.

ISO weeks are in common use throughout Europe, it's part of the
ISO 8601 standard. mxDateTime has had such constructors for ages:

http://www.egenix.com/products/python/mxBase/mxDateTime/doc/#_Toc293683820
msg157914 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-04-09 22:34
On Mon, Apr 9, 2012 at 6:20 PM, Marc-Andre Lemburg
<report@bugs.python.org> wrote:
> Which is wrong, since the start of the first ISO week of a year
> can in fact start in the preceeding year...

Hmm, the dateutil documentation seems to imply that relativedelta
takes care of this:

http://labix.org/python-dateutil#head-72c4689ec5608067d118b9143cef6bdffb6dad4e

(Search the page for "ISO")
msg157920 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-09 22:56
> Hmm, the dateutil documentation seems to imply that relativedelta
> takes care of this:

This is all a bit moot since we don't have a "relativedelta" in the
stdlib.
I think the two questions here are:
- is this feature desirable? I think the answer is "yes"
- is the implementation correct? I don't know :-)

Talk of a magical generic constructor sounds a bit futile to me,
especially if the constructor's behaviour becomes so complicated that
it's difficult to understand for users...
msg157922 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-04-09 23:01
On Mon, Apr 9, 2012 at 6:56 PM, Antoine Pitrou <report@bugs.python.org> wrote:
> This is all a bit moot since we don't have a "relativedelta" in the
> stdlib.

It is still worthwhile to see how it is done elsewhere.  So far, we
have dateutil that does not have this function and mxDateTime that
keeps this and other ISO related functions in a submodule.  Does
anyone know how this is done in other languages?
msg157924 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-09 23:28
Alexander Belopolsky wrote:
> 
> Alexander Belopolsky <alexander.belopolsky@gmail.com> added the comment:
> 
> On Mon, Apr 9, 2012 at 6:20 PM, Marc-Andre Lemburg
> <report@bugs.python.org> wrote:
>> Which is wrong, since the start of the first ISO week of a year
>> can in fact start in the preceeding year...
> 
> Hmm, the dateutil documentation seems to imply that relativedelta
> takes care of this:
> 
> http://labix.org/python-dateutil#head-72c4689ec5608067d118b9143cef6bdffb6dad4e
> 
> (Search the page for "ISO")

That's not realtivedelta taking care of it, it's the way it is
used: the week with 4.1. in it is the first ISO week of a year;
it then goes back to the previous Monday and adds 14 weeks from
there to go to the Monday of the 15th week. This works fine as
long as 4.1. doesn't fall on a Monday...

You don't really expect anyone to remember such rules, do you ? :-)
msg157933 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-04-10 02:56
On Mon, Apr 9, 2012 at 7:28 PM, Marc-Andre Lemburg
<report@bugs.python.org> wrote:
> You don't really expect anyone to remember such rules, do you ? :-)

No, but it is still a one-line function that those who need it can
easily implement.  I am on the fence here because we already have
date.isocalendar() function, so it is natural to desire its inverse,
but still at least on this side of the pond an Easter(year) date
constructor would see more use than that.
msg157941 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-04-10 09:48
I believe that it is a good solution to have, for lack of a better term;
bi-directional features so
in my opinion .isocalendar()  merits having a constructor that takes an ISO
format.

Sadly no :-(
I looked it over once more and it seems there is an error where the date is
not offset correctly
for years beginning on Tuesday, Wednesday or Thursday.
I will updater my patch ASAP.

+        if self.isocalendar()[1] != 1:
+            if self.weekday() > 3:  # Jan 1 is not in week one

+                 self += timedelta(7 - self.weekday())

+        else:

+            self -= timedelta(self.weekday())

----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue14423>
> _______________________________________
>
msg157945 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-10 10:44
> No, but it is still a one-line function that those who need it can
> easily implement.

It's so easy that the patch isn't a one-liner and it seems to still have
bugs wrt. intended behaviour.

>   I am on the fence here because we already have
> date.isocalendar() function, so it is natural to desire its inverse,
> but still at least on this side of the pond an Easter(year) date
> constructor would see more use than that.

This isn't an either/or situation. We can have both from_iso_week() and
Easter() if both are useful.
And I don't get this "side of the pond" argument. Python is not meant
only for the American public, that's why all strings are now unicode.
msg157951 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-04-10 13:32
On Tue, Apr 10, 2012 at 6:44 AM, Antoine Pitrou <report@bugs.python.org> wrote:
> It's so easy that the patch isn't a one-liner and it seems to still have
> bugs wrt. intended behaviour.

Unless I miss something, the inverse to isocalendar() is simply

from datetime import *

def fromiso(year, week, day):
    d = date(year, 1, 4)
    return d + timedelta((week - 1) * 7 + day - d.isoweekday())

At least it works in my testing:

(2012, 15, 2)
msg157983 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-04-10 20:52
1) Yes I agree, your solution is somewhat more concise, I have corrected
the code accordingly.
2) I get errors for all my test when I build "my" python and run
"./python.exe -m test.datetimetester -j3"
    I asume this is because I have yet to implement the c version in
Modules/_datetimemodule.c
    is this the correct assumption?

Kind regards

On Tue, Apr 10, 2012 at 3:32 PM, Alexander Belopolsky <
report@bugs.python.org> wrote:

>
> Alexander Belopolsky <alexander.belopolsky@gmail.com> added the comment:
>
> On Tue, Apr 10, 2012 at 6:44 AM, Antoine Pitrou <report@bugs.python.org>
> wrote:
> > It's so easy that the patch isn't a one-liner and it seems to still have
> > bugs wrt. intended behaviour.
>
> Unless I miss something, the inverse to isocalendar() is simply
>
> from datetime import *
>
> def fromiso(year, week, day):
>    d = date(year, 1, 4)
>    return d + timedelta((week - 1) * 7 + day - d.isoweekday())
>
> At least it works in my testing:
>
> (2012, 15, 2)
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue14423>
> _______________________________________
>
msg157985 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-10 20:58
> 1) Yes I agree, your solution is somewhat more concise, I have corrected
> the code accordingly.
> 2) I get errors for all my test when I build "my" python and run
> "./python.exe -m test.datetimetester -j3"
>     I asume this is because I have yet to implement the c version in
> Modules/_datetimemodule.c
>     is this the correct assumption?

You should probably run "./python.exe -m test -v test_datetime" instead.
Not sure that will fix your test failures, though :)
msg158710 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-04-19 09:27
>> 2) I get errors for all my test when I build "my" python and run
>> "./python.exe -m test.datetimetester -j3"
>>     I asume this is because I have yet to implement the c version in
>> Modules/_datetimemodule.c
>>     is this the correct assumption?

>You should probably run "./python.exe -m test -v test_datetime" instead.
>Not sure that will fix your test failures, though :)

Ok so now i only get errors for "_Fast" tests am I correct in assuming that this is because i lack a C implementation?
msg158981 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-04-22 18:53
> Ok so now i only get errors for "_Fast" tests
> am I correct in assuming that this is because i lack a C implementation?

If your errors look like the one below, then yes.


======================================================================
ERROR: test_from_iso_week_value_week (test.datetimetester.TestDate_Fast)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mdickinson/Python/cpython/Lib/test/datetimetester.py", line 1000, in test_from_iso_week_value_week
    self.assertRaises(ValueError, self.theclass.from_iso_week, year,
AttributeError: type object 'datetime.date' has no attribute 'from_iso_week'
msg158987 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-04-22 20:16
By the way, I don't think the algorithm used in the current patch is correct.  For 'date.from_iso_week(2009, 1)' I get 2009/1/1, which was a Thursday.  The documentation seems to indicate that a Monday should be returned.
msg158989 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2012-04-22 20:25
Mark Dickinson wrote:
> 
> By the way, I don't think the algorithm used in the current patch is correct.  For 'date.from_iso_week(2009, 1)' I get 2009/1/1, which was a Thursday.  The documentation seems to indicate that a Monday should be returned.

True, the correct date is 2008-12-29.
msg158990 - (view) Author: Esben Agerbæk Black (Esben.Agerbæk.Black) Date: 2012-04-22 21:12
I somehow re-uploaded an old patch,
this one contains buggy c-code, but fixes the python implementation.

Any pointers to how to get started on the c bi would be greatly appreciated.
msg162887 - (view) Author: Erik Cederstrand (Erik Cederstrand) Date: 2012-06-15 11:04
I would like to point out that http://bugs.python.org/issue12006 provides a solution (including patches) based on %G%, V and %u directives for use in strptime()/strftime(). These directives are defined in (FreeBSD) libc, and PHP has them, too.

I think the strptime approach is a more elegant and standardized way of creating dates from exotic values.
msg252166 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-10-02 22:00
I am rejecting this in favor of #12006.
History
Date User Action Args
2015-10-02 22:00:19belopolskysetstatus: open -> closed
superseder: strptime should implement %G, %V and %u directives
resolution: rejected
messages: + msg252166
2012-06-15 11:04:49Erik Cederstrandsetnosy: + Erik Cederstrand
messages: + msg162887
2012-04-22 21:12:45Esben.Agerbæk.Blacksetfiles: + isodates.patch

messages: + msg158990
2012-04-22 20:25:24lemburgsetmessages: + msg158989
2012-04-22 20:16:20mark.dickinsonsetmessages: + msg158987
2012-04-22 18:53:47mark.dickinsonsetnosy: + mark.dickinson
messages: + msg158981
2012-04-19 09:27:06Esben.Agerbæk.Blacksetmessages: + msg158710
2012-04-10 20:58:08pitrousetmessages: + msg157985
2012-04-10 20:52:31Esben.Agerbæk.Blacksetmessages: + msg157983
2012-04-10 13:32:30belopolskysetmessages: + msg157951
2012-04-10 10:44:40pitrousetmessages: + msg157945
2012-04-10 09:48:20Esben.Agerbæk.Blacksetmessages: + msg157941
2012-04-10 08:26:13hayposetnosy: - haypo
2012-04-10 02:56:45belopolskysetmessages: + msg157933
2012-04-09 23:28:42lemburgsetmessages: + msg157924
2012-04-09 23:01:45belopolskysetmessages: + msg157922
2012-04-09 22:56:24pitrousetmessages: + msg157920
2012-04-09 22:34:03belopolskysetmessages: + msg157914
2012-04-09 22:20:25lemburgsetmessages: + msg157912
2012-04-09 21:50:13belopolskysetmessages: + msg157907
2012-04-09 21:29:07pitrousetnosy: + pitrou
messages: + msg157905
2012-04-08 11:38:06Esben.Agerbæk.Blacksetfiles: + isodates.patch
2012-03-31 10:11:54Esben.Agerbæk.Blacksetfiles: + isodates.patch
2012-03-29 22:32:51pitrousetnosy: + lemburg, haypo
2012-03-28 18:39:42Esben.Agerbæk.Blacksetfiles: + isodates.patch

messages: + msg156997
2012-03-28 17:36:23r.david.murraysetkeywords: + needs review
nosy: + belopolsky
stage: patch review

versions: - Python 2.6, Python 3.1, Python 2.7, Python 3.2, Python 3.4
2012-03-28 17:20:46Esben.Agerbæk.Blacksetfiles: + isodates.patch

messages: + msg156991
2012-03-27 18:51:02Esben.Agerbæk.Blacksetcomponents: + Library (Lib)
2012-03-27 18:50:21Esben.Agerbæk.Blacksetfiles: + isodates.patch
keywords: + patch
messages: + msg156951
2012-03-27 18:46:44Esben.Agerbæk.Blacksetfiles: - weekdate.py
2012-03-27 18:46:26Esben.Agerbæk.Blacksethgrepos: - hgrepo117
2012-03-27 12:01:13Esben.Agerbæk.Blackcreate