This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author mark.dickinson
Recipients ajaksu2, akira, belopolsky, brett.cannon, daniel.urban, doerwalter, eric.araujo, ggenellina, kawai, l0nwlf, mark.dickinson, pitrou, r.david.murray, rafe, techtonik, tim.peters
Date 2010-06-13.14:09:06
SpamBayes Score 0.00072533137
Marked as misclassified No
Message-id <1276438148.67.0.549637021497.issue5094@psf.upfronthosting.co.za>
In-reply-to
Content
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?
History
Date User Action Args
2010-06-13 14:09:09mark.dickinsonsetrecipients: + mark.dickinson, tim.peters, doerwalter, brett.cannon, belopolsky, ggenellina, pitrou, techtonik, ajaksu2, kawai, eric.araujo, r.david.murray, rafe, daniel.urban, l0nwlf, akira
2010-06-13 14:09:08mark.dickinsonsetmessageid: <1276438148.67.0.549637021497.issue5094@psf.upfronthosting.co.za>
2010-06-13 14:09:07mark.dickinsonlinkissue5094 messages
2010-06-13 14:09:06mark.dickinsoncreate