Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

edited Apr 13, 2017 at 12:40

There's no docstring. What does the week_string function do, and how should I call it? In particular, what is the meaning of the dt argument?
You've put your test cases at top level in the script. This means that they get run whenever the script is loaded. It would be better to refactor the test cases into unit tests or doctests.
Your code is not portable to Python 3 because the test cases use the statement form of print.
You import datetime.datetime but don't use it. And you import datetime.date but only use it in the test cases.

There's no need for parentheses in these lines:

 from datetime import (datetime, date, timedelta)
 (year, week, weekday) = dt.isocalendar()

The variables are poorly named, as explained by unholysampler as explained by unholysampler.
You format the components (year, month, day) of the dates and then compare the formatted components:
```
 year_0 = week_0.strftime("%Y")
 year_1 = week_1.strftime("%Y")
 if year_0 != year_1:
 # ...
```

So I would follow unholysampler's technique unholysampler's technique and use Python's string formatting operation on the day field of the date objects.

Date-manipulating code is often tricky, and you made a mistake in the case where dt is on a Sunday, as pointed out by 200_success as pointed out by 200_success. So it would be worth putting in some assertions to check that the manipulations are correct. You can see in the revised code below that I've added assertions checking that begin is on a Sunday, that end is on a Saturday, and that d lies between these two dates.

There's no docstring. What does the week_string function do, and how should I call it? In particular, what is the meaning of the dt argument?
You've put your test cases at top level in the script. This means that they get run whenever the script is loaded. It would be better to refactor the test cases into unit tests or doctests.
Your code is not portable to Python 3 because the test cases use the statement form of print.
You import datetime.datetime but don't use it. And you import datetime.date but only use it in the test cases.

There's no need for parentheses in these lines:

 from datetime import (datetime, date, timedelta)
 (year, week, weekday) = dt.isocalendar()

The variables are poorly named, as explained by unholysampler.
You format the components (year, month, day) of the dates and then compare the formatted components:
```
 year_0 = week_0.strftime("%Y")
 year_1 = week_1.strftime("%Y")
 if year_0 != year_1:
 # ...
```

So I would follow unholysampler's technique and use Python's string formatting operation on the day field of the date objects.

Date-manipulating code is often tricky, and you made a mistake in the case where dt is on a Sunday, as pointed out by 200_success. So it would be worth putting in some assertions to check that the manipulations are correct. You can see in the revised code below that I've added assertions checking that begin is on a Sunday, that end is on a Saturday, and that d lies between these two dates.

There's no docstring. What does the week_string function do, and how should I call it? In particular, what is the meaning of the dt argument?
You've put your test cases at top level in the script. This means that they get run whenever the script is loaded. It would be better to refactor the test cases into unit tests or doctests.
Your code is not portable to Python 3 because the test cases use the statement form of print.
You import datetime.datetime but don't use it. And you import datetime.date but only use it in the test cases.

There's no need for parentheses in these lines:

 from datetime import (datetime, date, timedelta)
 (year, week, weekday) = dt.isocalendar()

The variables are poorly named, as explained by unholysampler.
You format the components (year, month, day) of the dates and then compare the formatted components:
```
 year_0 = week_0.strftime("%Y")
 year_1 = week_1.strftime("%Y")
 if year_0 != year_1:
 # ...
```

So I would follow unholysampler's technique and use Python's string formatting operation on the day field of the date objects.

Date-manipulating code is often tricky, and you made a mistake in the case where dt is on a Sunday, as pointed out by 200_success. So it would be worth putting in some assertions to check that the manipulations are correct. You can see in the revised code below that I've added assertions checking that begin is on a Sunday, that end is on a Saturday, and that d lies between these two dates.

Source Link

answered Jan 27, 2014 at 13:13

Gareth Rees

answered Jan 27, 2014 at 13:13

Gareth Rees

50.1k
3
130
210

1. Comments on your code

There's no docstring. What does the week_string function do, and how should I call it? In particular, what is the meaning of the dt argument?
You've put your test cases at top level in the script. This means that they get run whenever the script is loaded. It would be better to refactor the test cases into unit tests or doctests.
Your code is not portable to Python 3 because the test cases use the statement form of print.
You import datetime.datetime but don't use it. And you import datetime.date but only use it in the test cases.

There's no need for parentheses in these lines:

 from datetime import (datetime, date, timedelta)
 (year, week, weekday) = dt.isocalendar()

The variables are poorly named, as explained by unholysampler.
You format the components (year, month, day) of the dates and then compare the formatted components:
```
 year_0 = week_0.strftime("%Y")
 year_1 = week_1.strftime("%Y")
 if year_0 != year_1:
 # ...
```

but it would make more sense to compare the year property of the dates directly:

 if begin.year != end.year:
 # ...

The %e format code for srtftime was not defined by the C89 standard and so may not be portable to all platforms where Python runs. See the strftime documentation where this is noted. Also, even where implemented, the %e format code outputs a leading space which doesn't seem appropriate in your case.

So I would follow unholysampler's technique and use Python's string formatting operation on the day field of the date objects.

Date-manipulating code is often tricky, and you made a mistake in the case where dt is on a Sunday, as pointed out by 200_success. So it would be worth putting in some assertions to check that the manipulations are correct. You can see in the revised code below that I've added assertions checking that begin is on a Sunday, that end is on a Saturday, and that d lies between these two dates.

2. Revised code

from datetime import timedelta
def week_description(d):
 """Return a description of the calendar week (Sunday to Saturday)
 containing the date d, avoiding repetition.
 >>> from datetime import date
 >>> week_description(date(2013, 12, 30))
 'Dec 29, 2013 - Jan 4, 2014'
 >>> week_description(date(2014, 1, 25))
 'Jan 19 - 25, 2014'
 >>> week_description(date(2014, 1, 26))
 'Jan 26 - Feb 1, 2014'
 """
 begin = d - timedelta(days=d.isoweekday() % 7)
 end = begin + timedelta(days=6)
 assert begin.isoweekday() == 7 # Sunday
 assert end.isoweekday() == 6 # Saturday
 assert begin <= d <= end
 if begin.year != end.year:
 fmt = '{0:%b} {0.day}, {0.year} - {1:%b} {1.day}, {1.year}'
 elif begin.month != end.month:
 fmt = "{0:%b} {0.day} - {1:%b} {1.day}, {1.year}"
 else:
 fmt = "{0:%b} {0.day} - {1.day}, {1.year}"
 return fmt.format(begin, end)

lang-py