Re: [Python-Dev] [Python-checkins] r43545 - in python/trunk: Doc/lib/libcalendar.tex Lib/calendar.py
Author: walter.doerwald Date: Sat Apr 1 22:40:23 2006 New Revision: 43545
Modified: python/trunk/Doc/lib/libcalendar.tex python/trunk/Lib/calendar.py Log: Make firstweekday a simple attribute instead of hiding it behind a setter and a getter.
Walter, what's the purpose of this patch? The first weekday is still an attribute, but instead of being settable and gettable via methods, looks like it's now settable and gettable via module-global functions, and only for the single default instance of Calendar created by the module. If so, then (a) the functionality of the Calendar class is weaker now, and in a backward-incompatible way; and, (b) the new module-global firstweekday() and setfirstweekday() functions are a more obscure way to restore the lost functionality for just one specific instance of a Calendar subclass. I don't see the attraction to any part of this.
--- python/trunk/Lib/calendar.py (original) +++ python/trunk/Lib/calendar.py Sat Apr 1 22:40:23 2006 @@ -128,25 +128,14 @@ """
def __init__(self, firstweekday=0): - self._firstweekday = firstweekday # 0 = Monday, 6 = Sunday - - def firstweekday(self): - return self._firstweekday - - def setfirstweekday(self, weekday): - """ - Set weekday (Monday=0, Sunday=6) to start each week. - """ - if not MONDAY <= weekday <= SUNDAY: - raise IllegalWeekdayError(weekday) - self._firstweekday = weekday + self.firstweekday = firstweekday # 0 = Monday, 6 = Sunday
Removing those Calendar methods is backward-incompatible, ...
-firstweekday = c.firstweekday -setfirstweekday = c.setfirstweekday +def firstweekday(): + return c.firstweekday + +def setfirstweekday(firstweekday): + if not MONDAY <= firstweekday <= SUNDAY: + raise IllegalWeekdayError(firstweekday) + c.firstweekday = firstweekday + monthcalendar = c.monthdayscalendar prweek = c.prweek
And here they're obscurely added back again, but work only for the module-global default instance `c` of the TextCalendar subclass.
Tim Peters wrote:
Author: walter.doerwald Date: Sat Apr 1 22:40:23 2006 New Revision: 43545
Modified: python/trunk/Doc/lib/libcalendar.tex python/trunk/Lib/calendar.py Log: Make firstweekday a simple attribute instead of hiding it behind a setter and a getter.
Walter, what's the purpose of this patch? The first weekday is still an attribute, but instead of being settable and gettable via methods, looks like it's now settable and gettable via module-global functions, and only for the single default instance of Calendar created by the module. If so, then (a) the functionality of the Calendar class is weaker now, and in a backward-incompatible way; and, (b) the new module-global firstweekday() and setfirstweekday() functions are a more obscure way to restore the lost functionality for just one specific instance of a Calendar subclass.
I don't see the attraction to any part of this.
--- python/trunk/Lib/calendar.py (original) +++ python/trunk/Lib/calendar.py Sat Apr 1 22:40:23 2006 @@ -128,25 +128,14 @@ """
def __init__(self, firstweekday=0): - self._firstweekday = firstweekday # 0 = Monday, 6 = Sunday - - def firstweekday(self): - return self._firstweekday - - def setfirstweekday(self, weekday): - """ - Set weekday (Monday=0, Sunday=6) to start each week. - """ - if not MONDAY <= weekday <= SUNDAY: - raise IllegalWeekdayError(weekday) - self._firstweekday = weekday + self.firstweekday = firstweekday # 0 = Monday, 6 = Sunday
Removing those Calendar methods is backward-incompatible,
Isn't it that the Calendar class was just added and therefore is new to 2.5?
-firstweekday = c.firstweekday -setfirstweekday = c.setfirstweekday +def firstweekday(): + return c.firstweekday + +def setfirstweekday(firstweekday): + if not MONDAY <= firstweekday <= SUNDAY: + raise IllegalWeekdayError(firstweekday) + c.firstweekday = firstweekday + monthcalendar = c.monthdayscalendar prweek = c.prweek
And here they're obscurely added back again, but work only for the module-global default instance `c` of the TextCalendar subclass.
Since the functions are part of the traditional calendar API which cannot be broken. Georg
Tim Peters wrote:
Author: walter.doerwald Date: Sat Apr 1 22:40:23 2006 New Revision: 43545
Modified: python/trunk/Doc/lib/libcalendar.tex python/trunk/Lib/calendar.py Log: Make firstweekday a simple attribute instead of hiding it behind a setter and a getter.
Walter, what's the purpose of this patch? The first weekday is still an attribute, but instead of being settable and gettable via methods, looks like it's now settable and gettable via module-global functions, and only for the single default instance of Calendar created by the module.
This is because in 2.4 there where no Calendar objects and firstweekday was only setable and getable via module level functions.
If so, then (a) the functionality of the Calendar class is weaker now,
No really. firstweekday is changeable simply by assigning to the attribute: import calendar cal = calendar.Calendar() cal.firstweekday = 6 The only thing lost is the range check in the setter.
and in a backward-incompatible way;
Yes, this change is backwards-incompatible, but imcompatible to code that has been in the repository since Friday, so this shouldn't be a problem! ;)
and, (b) the new module-global firstweekday() and setfirstweekday() functions are a more obscure way to restore the lost functionality for just one specific instance of a Calendar subclass.
That's because in 2.4 the module level interface was all there was.
I don't see the attraction to any part of this.
Simple attribute access looks much more Pythonic to me than setters and gettes (especially as the attributes of subclasses are simple attributes). Or are you talking about the Calendar class itself?
[...]
Bye, Walter Dörwald
Walter Dörwald wrote:
firstweekday is changeable simply by assigning to the attribute:
import calendar cal = calendar.Calendar() cal.firstweekday = 6
The only thing lost is the range check in the setter.
Any particular reason for not making it a property? Then you could keep the range check while still manipulating it as if it was a simple attribute. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
[Tim, gripes about ...]
Author: walter.doerwald Date: Sat Apr 1 22:40:23 2006 New Revision: 43545
Modified: python/trunk/Doc/lib/libcalendar.tex python/trunk/Lib/calendar.py Log: Make firstweekday a simple attribute instead of hiding it behind a setter and a getter.
[Walter][
This is because in 2.4 there where no Calendar objects and firstweekday was only setable and getable via module level functions.
I didn't realize that, of course <blush>. Skipping the rest ;-), then, it would be best to make firstweekday a property on the new base class.
... The only thing lost is the range check in the setter.
Which isn't a good thing to lose. It's not good that the current Calendar constructor skips that sanity check either ("errors should never pass silently").
... Simple attribute access looks much more Pythonic to me than setters and gettes (especially as the attributes of subclasses are simple attributes). Or are you talking about the Calendar class itself?
Yes, it would be best if Calendar had a property, so that sanity checks were performed when setting `firstweekday`, and also if the Calendar constructor performed that sanity check (which could happen "by magic" if `firstweekday` were a property).
Tim Peters wrote:
[Tim, gripes about ...]
Author: walter.doerwald Date: Sat Apr 1 22:40:23 2006 New Revision: 43545
Modified: python/trunk/Doc/lib/libcalendar.tex python/trunk/Lib/calendar.py Log: Make firstweekday a simple attribute instead of hiding it behind a setter and a getter.
[Walter][
This is because in 2.4 there where no Calendar objects and firstweekday was only setable and getable via module level functions.
I didn't realize that, of course <blush>. Skipping the rest ;-), then, it would be best to make firstweekday a property on the new base class.
... The only thing lost is the range check in the setter.
Which isn't a good thing to lose. It's not good that the current Calendar constructor skips that sanity check either ("errors should never pass silently").
I've changed calendar so that firstweekday is only used modulo 7 everywhere (There was only one spot missing, all other cases used firstweekday modulo 7 anyway.
... Simple attribute access looks much more Pythonic to me than setters and gettes (especially as the attributes of subclasses are simple attributes). Or are you talking about the Calendar class itself?
Yes, it would be best if Calendar had a property, so that sanity checks were performed when setting `firstweekday`, and also if the Calendar constructor performed that sanity check (which could happen "by magic" if `firstweekday` were a property).
Range checks should no longer be neccessary, as any value works now. Bye, Walter Dörwald
Walter Dörwald wrote:
Tim Peters wrote:
Which isn't a good thing to lose. It's not good that the current Calendar constructor skips that sanity check either ("errors should never pass silently").
I've changed calendar so that firstweekday is only used modulo 7 everywhere (There was only one spot missing, all other cases used firstweekday modulo 7 anyway.
... Simple attribute access looks much more Pythonic to me than setters and gettes (especially as the attributes of subclasses are simple attributes). Or are you talking about the Calendar class itself? Yes, it would be best if Calendar had a property, so that sanity checks were performed when setting `firstweekday`, and also if the Calendar constructor performed that sanity check (which could happen "by magic" if `firstweekday` were a property).
Range checks should no longer be neccessary, as any value works now.
But now all *clients* of the Calendar class are forced to deal with the fact that "firstweekday" may not be greater than seven. If you want to accept any input value, why not use a property to force it to be modulo 7, rather than doing an actual range check? Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Nick Coghlan wrote:
Walter Dörwald wrote:
[...] Range checks should no longer be neccessary, as any value works now.
But now all *clients* of the Calendar class are forced to deal with the fact that "firstweekday" may not be greater than seven.
If you want to accept any input value, why not use a property to force it to be modulo 7, rather than doing an actual range check?
OK, the property setter does a "% 7" now. (But the global setfirstweekday() still does a range check). Bye. Walter Dörwald
Walter Dörwald wrote:
OK, the property setter does a "% 7" now. (But the global setfirstweekday() still does a range check).
Wouldn't it be better for the setter to raise an exception if it's out of range? It probably indicates a bug in the caller's code. -- Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | Carpe post meridiam! | Christchurch, New Zealand | (I'm not a morning person.) | greg.ewing@canterbury.ac.nz +--------------------------------------+
Greg Ewing wrote:
Walter Dörwald wrote:
OK, the property setter does a "% 7" now. (But the global setfirstweekday() still does a range check).
Wouldn't it be better for the setter to raise an exception if it's out of range? It probably indicates a bug in the caller's code.
The day before Monday is -1, so it adds a little convenience. If this convenience is really worth it is a completely different topic. I think we've spent more time discussing the calendar module, than the Python community has spent using it! ;) Bye, Walter Dörwald
Walter Dörwald wrote:
Greg Ewing wrote:
Wouldn't it be better for the setter to raise an exception if it's out of range? It probably indicates a bug in the caller's code.
The day before Monday is -1, so it adds a little convenience.
In that case, why doesn't the global function allow the same convenience?
I think we've spent more time discussing the calendar module, than the Python community has spent using it! ;)
It makes a nice change from adaptation and generic functions. :-) -- Greg
Greg Ewing wrote:
Walter Dörwald wrote:
Greg Ewing wrote:
Wouldn't it be better for the setter to raise an exception if it's out of range? It probably indicates a bug in the caller's code.
The day before Monday is -1, so it adds a little convenience.
In that case, why doesn't the global function allow the same convenience?
For backwards compatibility reasons.
I think we've spent more time discussing the calendar module, than the Python community has spent using it! ;)
It makes a nice change from adaptation and generic functions. :-)
;) Bye, Walter Dörwald
participants (5)
-
Georg Brandl -
Greg Ewing -
Nick Coghlan -
Tim Peters -
Walter Dörwald