calendar.isleap() doesn't enforce any types, leading to string formatting errors when a string is passed to it:

>>> calendar.isleap("2011")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.1/lib/python3.1/calendar.py", line 99, in isleap
    return year % 4 == 0 and (year % 100 != 0 or year % 400 == 0)
TypeError: not all arguments converted during string formatting

A quick peek at the SVN shows that the problem still exists.

In Python we often don't type check, we just let errors happen.  It is true that it would make the problem clearer to do a type check and issue a specific error message, but I don't know if it is worth it.  (The error would already have been clear if it weren't for the fact that % is the string formatting operator...)

I agree with David on this one.  In addition to the arguments already stated, the string value of an exception is not part of specification.  The exception is still TypeError even if the message may be confusing.  I also find that once read in the context of the backtrace, the error is clear enough.

I am changing this to an RFE for now, but I think this should be closed as "won't fix".

I would also like to note the following curiosity:

>>> calendar.isleap("%d")
False

I still don't think there is anything worth fixing here, but we can consider replacing year % 4 == 0 predicate with year & 3 == 0 which will change the error raised by isleap("2004") from

TypeError: not all arguments converted during string formatting

to 

TypeError: unsupported operand type(s) for &: 'str' and 'int'

I am not sure if the later is much clearer than the former once you consider obfuscated year & 3 == 0 showing up in the backtrace compared to a clear year % 4 == 0 test.

I’d be in favor of type checking here, but I know I’m in the minority.  I’m very much not in favor of making code less readable (“& 3”).

To clarify my original point, I'm for making an explicit error message for that, definitely not for any silent failure or automatic string conversion.

Another option is to wrap the operations in a try/except. When a TypeError is raised have it throw a new TypeError with an improved error message and the original chained to it

I don't see the point.  If you file one such bug per day, you won't be finished in a year -- there's no reason to start adding typechecking in this function.

To me, Alexander's example
  >>> calendar.isleap("%d")
  False
is a buggy result. So I would reclassify the issue.

The rationale for not checking input types is that bad types result in an error, but that does not happen here due to a design decision that some consider clever and some buggy, and some both. (Guido himself would like to deprecate it.)

I am in favor of the 'year & 3 == 0' fix so that any input string (and indeed, anything that does not implement both % and & operators) raises an error. Intermediate to expert programmers should know or learn about bit masking. Add a comment to explain the reason -- and a test case.

> To me, Alexander's example
>   >>> calendar.isleap("%d")
>   False
> is a buggy result. So I would reclassify the issue.

You'll always find plenty "wrong" inputs for which a function doesn't
raise an exception.  That's the downside of duck typing.

> The rationale for not checking input types is that bad types result
> in an error, but that does not happen here due to a design decision
> that some consider clever and some buggy, and some both. (Guido
> himself would like to deprecate it.)

You're talking of % string formatting?  Well, that's just one operation
where duck typing applies.  There may be a bit less chance of namespace
collisions when calling methods, but it's the same issue at heart.

> I am in favor of the 'year & 3 == 0' fix so that any input string
> (and indeed, anything that does not implement both % and & operators)
> raises an error. Intermediate to expert programmers should know or
> learn about bit masking. Add a comment to explain the reason -- and
> a test case.

I agree that ``year & 3 == 0`` in this place is okay, and if it helps
avoid confusion, all the better.

Please leave this function as is because it works just fine.

>>> calendar.isleap(2011)
False


The argument for isleap() must not be in quotes. That's all.

I would just add an exception to the isleap() function.

def isleap(year):
    """Return 1 for leap years, 0 for non-leap years."""
    try:
        return year % 4 == 0 and (year % 100 != 0 or year % 400 == 0)
    except TypeError:
        # somehow inform the user that the argument 'year' must be an integer

Let me fix this function a little bit...

def isleap(year):
    """Return True for leap years, False for non-leap years."""
    if year == 0:
        raise ValueError('year 0 does not exist')
    return (year % 4 == 0) and (year % 100 != 0) or (year % 400 == 0)

This function, however, does not mind if you give it a negative number. But I think we can leave this option to mean B.C. (Before Christ), so calendar.isleap(-240) would mean 240 B.C., which was a leap year.

About the  if year == 0  check... Well, read Wikipedia's article  http://en.wikipedia.org/wiki/0_(year)  which clearly states that "Year zero does not exist in the widely used Gregorian calendar or in its predecessor, the Julian calendar."  So we raise a ValueError if this value is used in the calendar.isleap() function.

I have uploaded a patch that fixes this function. Please apply it to the trunk and also to the maintenance brances.

On Sun, Oct 17, 2010 at 3:54 PM, Boštjan Mejak <report@bugs.python.org> wrote:
..
> About the  if year == 0  check... Well, read Wikipedia's article  http://en.wikipedia.org/wiki/0_(year)  which clearly
> states that "Year zero does not exist in the widely used Gregorian calendar or in its predecessor, the Julian
> calendar."  So we raise a ValueError if this value is used in the calendar.isleap() function.
>

Well, Wikipedia is not the most authoritative source, but if you go
the right article there, you will find that

"""
Mathematically, it is more convenient to include a year zero and
represent earlier years as negative, for the specific purpose of
facilitating the calculation of the number of years between a negative
(BC) year and a positive (AD) year. This is the convention used in
astronomical year numbering and in the international standard date
system, ISO 8601. In these systems, the year 0 is a leap year.[2]
"""

Python documentation states that calendar module implements 'the
“proleptic Gregorian” calendar in Dershowitz and Reingold’s book
“Calendrical Calculations”'.  See
http://docs.python.org/dev/py3k/library/calendar.html

I don't have Dershowitz and Reingold’s book to check, but ISO/FDIS
8601:2000(E) standard contains a note that states: "In the prolaptic
[sic] Gregorian calendar the calendar year [0000] is a leap year."

In any case, I think there are more important issues with the calendar
module than perceived bugs in isleap() function.  See for example
issue 10087.

The most pedantic implementation of calendar.isleap() would be

from datetime import date, timedelta
def isleap(year):
    return date(year, 3, 1) - date(year, 2, 1) == timedelta(29)

Since python calendar only supports years in the range [1, 9999], the above will properly raise ValueError: year is out of range on negative or zero year.  This also guarantees that calendar module's notion of leap year is the same as that of datetime module.

If this is found to be a worthwhile change, I would also rewrite monthrange as

def monthrange(year, month):
    first = date(year, month, 1)
    if month == 12:
        ndays = 31
    else:
        ndays = (date(year, month + 1, 1) - first).days
    return first.weekday(), ndays

Also, your "pedantic" version of isleap() is not pedantic at all.

return date(year, 3, 1) - date(year, 2, 1) == timedelta(29)
does not seem readable at all. Readability counts!

return date(year, 3, 1) is not understandable. What are the arguments 3 and
1 in the date() function for?

On Mon, Oct 18, 2010 at 8:06 PM, Boštjan Mejak <bostjan.mejak@gmail.com>wrote:

>     else:
> -->    ndays = (date(year, month + 1, 1) - first).days
>     return first.weekday(), ndays
>
> Oh my God! The line with a pointer is so ugly!
>
> On Mon, Oct 18, 2010 at 7:59 PM, Alexander Belopolsky <
> report@bugs.python.org> wrote:
>
>>
>> Alexander Belopolsky <belopolsky@users.sourceforge.net> added the
>> comment:
>>
>> The most pedantic implementation of calendar.isleap() would be
>>
>> from datetime import date, timedelta
>> def isleap(year):
>>    return date(year, 3, 1) - date(year, 2, 1) == timedelta(29)
>>
>> Since python calendar only supports years in the range [1, 9999], the
>> above will properly raise ValueError: year is out of range on negative or
>> zero year.  This also guarantees that calendar module's notion of leap year
>> is the same as that of datetime module.
>>
>> If this is found to be a worthwhile change, I would also rewrite
>> monthrange as
>>
>> def monthrange(year, month):
>>    first = date(year, month, 1)
>>    if month == 12:
>>        ndays = 31
>>    else:
>>        ndays = (date(year, month + 1, 1) - first).days
>>    return first.weekday(), ndays
>>
>> ----------
>>
>> _______________________________________
>> Python tracker <report@bugs.python.org>
>> <http://bugs.python.org/issue10073>
>> _______________________________________
>>
>
>

> return date(year, 3, 1) is not understandable. What are the arguments 3 and
> 1 in the date() function for?

Boštjan, we appreciate your concern for the programming style of the
Python standard library.  However, your question shows that you should
probably spend a bit more time reading documentation than "fixing" code.

else:
-->    ndays = (date(year, month + 1, 1) - first).days
    return first.weekday(), ndays

Oh my God! The line with a pointer is so ugly!

On Mon, Oct 18, 2010 at 7:59 PM, Alexander Belopolsky <
report@bugs.python.org> wrote:

>
> Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment:
>
> The most pedantic implementation of calendar.isleap() would be
>
> from datetime import date, timedelta
> def isleap(year):
>    return date(year, 3, 1) - date(year, 2, 1) == timedelta(29)
>
> Since python calendar only supports years in the range [1, 9999], the above
> will properly raise ValueError: year is out of range on negative or zero
> year.  This also guarantees that calendar module's notion of leap year is
> the same as that of datetime module.
>
> If this is found to be a worthwhile change, I would also rewrite monthrange
> as
>
> def monthrange(year, month):
>    first = date(year, month, 1)
>    if month == 12:
>        ndays = 31
>    else:
>        ndays = (date(year, month + 1, 1) - first).days
>    return first.weekday(), ndays
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10073>
> _______________________________________
>

I don't think anything good will come out of this.  Closing as "rejected."

I have uploaded a new patch. It removes the  if year == 0  nonsense. Check it out. I hope you like it now.

When you were organizing the Standard Library, you didn't fix flaws in the calendar module. What I have in mind is, for example, the isleap() function which should be defined as is_leap(). But now it's all too late for that fixes.

Please apply my patch for calendar.isleap() to the trunk. Thanks.

Not only is this issue already closed, the patch is also wrong -- your change to the parentheses changes the semantics.

> for example, the isleap() function which should be defined as is_leap()
Who says that?  Not PEP 8: “Function names should be lowercase, with words separated by underscores *as necessary to improve readability*” (emphasis mine).  isleap is perfectly readable.

> But now it's all too late for that fixes.
You’ll find that Python has a balance between readability/ease of use and pragmatism.  In some cases, changes can’t be made, but it’s best to accept it and move on to the next issue.  Working on feature requests instead of style issues is great.

On a closing note, I’ll let you enjoy the Zen of Python:
$ python -m this

I have read the Zen of PYthon before. I have also written a typo report. But
got rejected. No one listens to my ideas. So why do you have a bug tracker
anyway?

On Tue, Oct 19, 2010 at 6:46 AM, Éric Araujo <report@bugs.python.org> wrote:

>
> Éric Araujo <merwok@netwok.org> added the comment:
>
> > for example, the isleap() function which should be defined as is_leap()
> Who says that?  Not PEP 8: “Function names should be lowercase, with words
> separated by underscores *as necessary to improve readability*” (emphasis
> mine).  isleap is perfectly readable.
>
> > But now it's all too late for that fixes.
> You’ll find that Python has a balance between readability/ease of use and
> pragmatism.  In some cases, changes can’t be made, but it’s best to accept
> it and move on to the next issue.  Working on feature requests instead of
> style issues is great.
>
> On a closing note, I’ll let you enjoy the Zen of Python:
> $ python -m this
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10073>
> _______________________________________
>

On Tue, Oct 19, 2010 at 12:34 PM, Boštjan Mejak <report@bugs.python.org> wrote:
> I have also written a typo report. But
> got rejected. No one listens to my ideas. So why do you have a bug tracker
> anyway?

Bug tracker is not the place to discuss ideas.  We have python-ideas
mailing list for that.

I searched the tracker and found four documentation issues that you
submitted in the past: #4389, #4648, #4649, and #6475.  In each case,
your issue received attention from the most senior python developers.
In each case the issue was closed with a detailed comment explaining
the reasons for rejection.

I think if you search the tracker for issues with "accepted" or
"fixed" resolution, you will understand why we have the bug tracker.
Reading successful reports may also help you to submit better reports
in the future.

>
> your change to the parentheses changes the semantics. (Georg Brandl)

How does adding those parens change semantics? The behaviour of the function
isleap() is the same. I have throughtly tested this function and found no
semantic errors in it. So the parens don't matter for behaviour, only for
clarity they matter. But since you decided not to fix this, I won't push the
envelope. At least fix the docstrings to match the return value. Please fix
the docstrings to """Return True for leap years, False for non-leap
years."""

On Tue, Oct 19, 2010 at 7:02 PM, Alexander Belopolsky <
report@bugs.python.org> wrote:

>
> Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment:
>
> On Tue, Oct 19, 2010 at 12:34 PM, Boštjan Mejak <report@bugs.python.org>
> wrote:
> > I have also written a typo report. But
> > got rejected. No one listens to my ideas. So why do you have a bug
> tracker
> > anyway?
>
> Bug tracker is not the place to discuss ideas.  We have python-ideas
> mailing list for that.
>
> I searched the tracker and found four documentation issues that you
> submitted in the past: #4389, #4648, #4649, and #6475.  In each case,
> your issue received attention from the most senior python developers.
> In each case the issue was closed with a detailed comment explaining
> the reasons for rejection.
>
> I think if you search the tracker for issues with "accepted" or
> "fixed" resolution, you will understand why we have the bug tracker.
> Reading successful reports may also help you to submit better reports
> in the future.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10073>
> _______________________________________
>

I am applying a patch that only fixes the docstrings to match the return value of calendar.isleap(). The return value of isleap() is not 1 or 0 -- it is True or False. Please apply the patch. Thank you.

Committed the docstring patch in r85725.

Thank you for applying my patch. Glad to help. Please apply it to the trunk and other branches as well.

It will be merged in due time.

> How does adding those parens change semantics?
It changes boolean evaluation paths.  See for example http://docs.python.org/py3k/reference/expressions#boolean-operations .  If you have more questions, python-list is an open and friendly mailing list dedicated to answering them.