__getitem__ & slice question
I checked in what I thought was a fix for calendar.py and test_calendar.py, then realized it didn't handle slices. I've never dealt with __getitem__ and slices before. Would someone try out this modified version of calendar._localized_name and let me know what I'm doing wrong? class _localized_name: def __init__(self, format, len): self.data = [] for i in range(len): self.data.append(strftime(format, (i,)*9).capitalize()) # months are one-based - match 2.1 behavior if format == '%b': self.data[0] = " " elif format == '%B': self.data[0] = "" def __getitem__(self, item): if type(item) is types.SliceType: return self.data[item.start:item.stop] return self.data[item] def __len__(self): return len(self.data) For example, try something like calendar.month_abbr[-20:] Also, should I worry about the slice's step attribute? Skip
[Skip Montanaro]
I checked in what I thought was a fix for calendar.py and test_calendar.py, then realized it didn't handle slices.
Why does that matter? calendar.__all__ doesn't export _localized_name or any of the gimmicks built from it, so it's purely an internal issue, and the internals never need slices here (granted that day_name etc are poorly named for module privates -- or perhaps the __all__ list and the docs are in error).
I've never dealt with __getitem__ and slices before. Would someone try out this modified version of calendar._localized_name and let me know what I'm doing wrong?
class _localized_name: def __init__(self, format, len): self.data = [] for i in range(len): self.data.append(strftime(format, (i,)*9).capitalize()) # months are one-based - match 2.1 behavior if format == '%b': self.data[0] = " " elif format == '%B': self.data[0] = ""
Now this changes the current semantics in a major way: if you're going to precompute all the values, construct plain tuples instead and be done with it. I assume that whoever patched this to begin with deliberately recomputed localized values on each dynamic reference so that they would respond appropriately to dynamic locale changes.
def __getitem__(self, item): if type(item) is types.SliceType: return self.data[item.start:item.stop] return self.data[item]
def __len__(self): return len(self.data)
For example, try something like
calendar.month_abbr[-20:]
Also, should I worry about the slice's step attribute?
To both, I don't think you should be changing this code to do static precomputation at all. But, if we must, toss the class and build plain tuples instead, e.g. day_name = tuple([strftime('%A', (i,)*9) for i in range(7)]) In the static case, trying to build a class that emulates a tuple is pointless.
>> I checked in what I thought was a fix for calendar.py and >> test_calendar.py, then realized it didn't handle slices. Tim> Why does that matter? calendar.__all__ doesn't export Tim> _localized_name or any of the gimmicks built from it, so it's Tim> purely an internal issue, and the internals never need slices here Tim> (granted that day_name etc are poorly named for module privates -- Tim> or perhaps the __all__ list and the docs are in error). Correct, but people still know they exist. I've used them a lot in my own code. It's just "well known" that they contain the weekday and month names. They were traditionally lists, so in my mind, they should probably try and behave as much like lists as is reasonable. Maybe they should be documented and exported in __all__, but that's a separate question. I'm not sure I've ever used the objects the module actually advertises except when working on the module's code. ;-) >> I've never dealt with __getitem__ and slices before. Would someone try >> out this modified version of calendar._localized_name and let me know >> what I'm doing wrong? >> >> class _localized_name: >> ... Tim> Now this changes the current semantics in a major way: if you're Tim> going to precompute all the values, construct plain tuples instead Tim> and be done with it. I assume that whoever patched this to begin Tim> with deliberately recomputed localized values on each dynamic Tim> reference so that they would respond appropriately to dynamic Tim> locale changes. Thanks, I hadn't considered that the locale might change dynamically. The above code is not what's checked in, however. The code I checked in still calls strftime from __getitem__. I also realized later (after the checkin) that the original SF submission was a patch submission, not a bug report that didn't include a fix. (When you visit patches or bugs by clicking on links in email messages they look just about the same.) Accordingly, there was some example code there to get me over the __getslice__ hump. Skip
Why does that matter? calendar.__all__ doesn't export _localized_name or any of the gimmicks built from it, so it's purely an internal issue, and the internals never need slices here (granted that day_name etc are poorly named for module privates -- or perhaps the __all__ list and the docs are in error).
I agree with Skip that these are public enough to make sure common uses will continue to work, despite their current lack of documentation. After all someone did report a bug -- I'm sure that was because they had real code using this which broke, not because they happened to have inspected the code and noticed it was buggy. :-) I have to say that I still think that adding __all__ to every module under the sun was a mistake. --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido> I have to say that I still think that adding __all__ to every Guido> module under the sun was a mistake. I said that while I was in the midst of adding __all__ to everything! Nobody listened... :-( Skip
Skip Montanaro writes:
I said that while I was in the midst of adding __all__ to everything! Nobody listened... :-(
Feel free to remove any that are specifically found to get in the way. Just tossing them all would probably not help; I think it makes more sense to do this as bugs get reported that are specifically fixed by removing them. Anytime we feel the need to add to __all__ is probably a good time to remove an __all__. ;-) -Fred -- Fred L. Drake, Jr. <fdrake at acm.org> PythonLabs at Zope Corporation
[Skip Montanaro]
I said that while I was in the midst of adding __all__ to everything! Nobody listened... :-(
I questioned it frequently <wink>. [Fred Drake, Jr.]
Feel free to remove any that are specifically found to get in the way. Just tossing them all would probably not help; I think it makes more sense to do this as bugs get reported that are specifically fixed by removing them. Anytime we feel the need to add to __all__ is probably a good time to remove an __all__. ;-)
-1. For good or ill, the mechanism is there now, and is the only we have to distinguish intended exports from accidental exports, given that nobody dared do the truly sane thing (renaming accidentally exported names to use a leading underscore).
participants (4)
-
Fred L. Drake, Jr. -
Guido van Rossum -
Skip Montanaro -
Tim Peters