Mark Dickinson <[email protected]> added the comment:
The latest patch looks good to me. I only have minor comments, in random order:
- Should the PyDateTime_TimeZone struct definition go into datetime.h, so that
it's avaiable if you want to export any C-API functions later on?
- If you're not allowing subclassing, then presumably you don't need
the new_timezone / new_timezone_ex dance?
- For clarity, please consider adding parentheses in:
self = (PyDateTime_TimeZone *)type->tp_alloc(type, 0);
to make it:
self = (PyDateTime_TimeZone *)(type->tp_alloc(type, 0));
- Whitespace issues: there are a couple of tabs in the source (at around lines
810, 3388, 3390), and an overly long line (>79 characters) at around line 3365.
- Please add a brief comment before the added C functions (like
new_timezone_ex) explaining their purpose.
- I wonder whether __ne__ should return the correct result (rather than
returning NotImplemented) for timezone instances. I realize that it's not
necessary to implement it in order for 'tz1 != tz2' to work, but it still makes
me uncomfortable that it's not implemented. OTOH, I agree with the decision
not to allow timezone order comparisons (though I bet they get requested at
some point).
- There doesn't seem to be any mention of timezone.min or timezone.max in the
docs. Is this deliberate?
----------
_______________________________________
Python tracker <[email protected]>
<http://bugs.python.org/issue5094>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
