Alexander Belopolsky <belopol...@users.sourceforge.net> added the comment:
On Fri, Jun 11, 2010 at 10:23 AM, Mark Dickinson <rep...@bugs.python.org> wrote: .. > Some comments from playing with this patch (without having looked at the > implementation): > thank you very much for your comments. As we are fine-tuning the timezone class, do you think it would be helpful to go back to Brett's suggestion and add datetime.py with just from _datetime import * and keep parallel Python and C implementations of timezone class only until full datetime.py is available? I was deliberate in a sense that I thought about it, but there was no substantial discussion. Original patch did not allow subclassing of the UTC class and I thought it was right. (See msg106415 and msg106422.) subclassing FixedOffsetTimeZone makes more sense, but I was hoping to keep timezone complete and eventually stop supporting tzinfo API (not allow passing datetime in the methods.) More on this below. I would very much prefer not to open the subclassing debate. If you support subclassing, I will add a flag. My only objection is that it is much easier to add this functionality in the future than to remove it if it gets abused. Can we postpone this decision until real life use case comes in? (Subclass to change dst() in itself is not a use case. Why do you need legacy timetuple interface to work is the first question.) > - If I try to do timezone(timedelta(hours=24)), I get an error message: > "ValueError: offset must be a timedelta between timedelta(1) and > -timedelta(1)." and I have to think for a bit to > remember that 'timedelta(1)' means 'timedelta(days=1)'. Any chance of making > this more explicit: e.g. "between > timedelta(hours=-24) and > timedelta(hours=24)"? > Please make a call and I'll implement it. The competing spellings are: 1. timedelta(hours=24) 2. 24 hours (I rejected this because it may be confused with an int) 3. timedelta(days=1) 4. in the range [-timedelta(hours=23, minutes=59), timedelta(hours=23, minutes=59)] Yes, there was a reason, but I don't think it is a good one. The default implementation of fromutc() does not work if utcoffset() returns timedelta but dst() returns None. The dst() value used to detect DST ambiguities. Remember, fromutc() despite the name is defined to operate on local times and some local times are ambiguous and some are invalid. The default implementation attempts to detect that. This is the issue that I am trying to avoid in my design. The test is easy to add. My longer term plan is to disallow passing argument to constant methods, so adding check is somewhat wasteful. Will do. > - And it also seems clunky that an argument *has* to be supplied for > utcoffset, tzname and dst, only to be i > gnored. Would it be possible to make the argument optional? Yes. this was my transition plan. Make it optional. Modify datetime methods to first try to get offset/name/dst without passing time argument and fall back to passing self. Remove optional argument from timezone methods. Yes. I want parseable __repr__ as well. I've been torn between 'datetime.timezone(datetime.timedelta(..)[, ..])' and 'timezone(timedelta(..)[, ..])'. Opinions welcome. Similarly, should str(timezone.utc) be '+0000' or 'UTC' or '+00:00'? You are reading my mind. I planned to tear out conditional logic that supports plain 'UTC', but apparently forgot. Will do. > - I'm very confused about utcoffset: why can't I supply a UTC datetime (i.e. > an aware datetime with > tzinfo = timezone.utc) to this? I suspect I'm misunderstanding something > here... You are not alone! :-) My understanding is that utcoffset was designed around the notion that if you add a day to 12 noon the day before DST change, you should still get 12 noon the next day. Supporting this requires deriving local timezone from local time which is impossible for some datetime values. Most business applications can ignore this because nobody schedules meetings between 1 and 2 am and those who do are smart enough to catch an exception and ask the user to clarify what time he/she means. Good catch. Thanks for the review. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue5094> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com