Is this pylint error message valid or bogus?

The vast majority of pythoners use None, but I prefer descriptive strings for default values, like this:

from datetime import datetime, timedelta

def midnight_next_day(initial_time="use today's date"):

    if initial_time == "use today's date":
        initial_time = datetime.now()

    return initial_time.date() + timedelta(days=1)

Pylint doesn’t like it though:


$ pylint -e f.py 
No config file found, using default configuration
************* Module f
E: 10:midnight_next_day: Instance of 'str' has no 'date' member (but some types could not be inferred)

I changed from using a string as the default to None, and then pylint
didn’t mind:


$ cat f.py
from datetime import datetime
def midnight_next_day(initial_time=None):

if initial_time is None:
    initial_time = datetime.now()

return initial_time.date() + timedelta(days=1)

$ pylint -e f.py
No config file found, using default configuration

Checking where a variable is None is probably much faster than doing string comparison, but I find the string more informative.

Looking for comments. I already posted this to comp.lang.python here and got some good feedback.

Posted in Programming, Python.

By matt

June 19, 2009

Comments

Viewing 8 Comments

Dave Kirby 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

I agree with the commenters on clp - as a user of the function looking at the signature, I would expect that initial_time needed to be a string.

If you wanted to use something more explicit than None, then you could use a placeholder object, e.g.

TODAY = object()
def midnight_next_day(initial_time=TODAY):
if initial_time is TODAY:
...

however pylint still complains that instance of object does not have a date method.

reply edit reblog flag

Ben Finney 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

> The vast majority of pythoners use None, but I prefer descriptive strings for default values

If you do that, you're communicating badly.

Python's default arguments mean something very specific, and readers will expect it: “this argument can be omitted, and if so, *this value will be used instead*”. By putting a value there that you never intend on using, you subvert that communication.

Please, if you don't have a good static value to use, just stick with the conventional sentinel value: None.

If you need None for some other purpose (e.g. you need to tell the difference between the default and a user-supplied None), use a clearly-for-no-other-purpose object, such as a module-local name bound to a unique object() instance.

reply edit reblog flag

Gary Bernhardt 9 months ago 2 points
Please login to rate.

Do you already have an account? Log in and claim this comment.

I'd turn it into two functions: midnight_tomorrow() and midnight_after(initial_date). Default arguments tell you that you're probably violating the SRP. (Or maybe they're *always* violations; I haven't thought enough about it yet.) Here's a version with two functions:

def midnight_tomorrow():
....return midnight_after(datetime.now())

def midnight_after(initial_time):
....return initial_time.date() + timedelta(days=1)

There are no conditionals needed, it's two eloc instead of three, and there's less thinking required by the guy who calls these functions. The caller doesn't need the docstring suggested above by Alex or the descriptive default value you used; the functions' names are fully self-descriptive.

reply edit reblog flag

http://blog.extracheese.org/

D'gou 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

Nicely refactored.
"Gordian knot? What gordian knot? Just a bunch of pieces of rope on the floor here." :)

reply edit reblog flag

nes 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

code does not come out that nice on the comment. Let's try this instead:

http://pastebin.com/m19049702

reply edit reblog flag

nes 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

pylint thinks initial_time might be a string sometimes.

>>> def midnight_next_day(initial_time="use today's date"):

if initial_time == "use today's date":
initial_time = datetime.now()

return initial_time.date() + timedelta(days=1)

>>> midnight_next_day()

Traceback (most recent call last):
File "<pyshell#24>", line 1, in <module>
midnight_next_day()
File "<pyshell#23>", line 6, in midnight_next_day
return initial_time.date() + timedelta(days=1)
AttributeError: 'str' object has no attribute 'date'
>>>

reply edit reblog flag

Alex 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

Sorry, but that's so nasty. Just use None and provide the description as a comment or in the docstring, please. None exists for a reason.

reply edit reblog flag

Anonymous 9 months ago 1 point
Please login to rate.

Do you already have an account? Log in and claim this comment.

there is no need for an if statement:

initial_time = intial_time or datatime.now()

the original version is bad because it suggests that your functions is taking string parameters, where in fact it takes datetime objects. Misleading.

reply edit reblog flag