Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Is this pylint error message valid or silly?

934 views
Skip to first unread message

Matthew Wilson

unread,
Jun 18, 2009, 8:56:18 PM6/18/09
to
Here's the code that I'm feeding to pylint:

$ cat f.py
from datetime import datetime

def f(c="today"):

if c == "today":
c = datetime.today()

return c.date()


And here's what pylint says:

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

Is this a valid error message? Is the code above bad? If so, what is
the right way?

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 f(c=None):

if c is None:
c = datetime.today()

return c.date()

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

I don't see any difference between using a string vs None. Both are
immutable. I find the string much more informative, since I can write
out what I want.

Looking for comments.

Matt

Ben Finney

unread,
Jun 19, 2009, 12:33:25 AM6/19/09
to
Matthew Wilson <ma...@tplus1.com> writes:

> Here's the code that I'm feeding to pylint:
>
> $ cat f.py
> from datetime import datetime
>
> def f(c="today"):
>
> if c == "today":
> c = datetime.today()
>
> return c.date()
>
>
> And here's what pylint says:
>
> $ pylint -e f.py
> No config file found, using default configuration
> ************* Module f
> E: 10:f: Instance of 'str' has no 'date' member (but some types could
> not be inferred)
>
> Is this a valid error message?

Yes. Mentally run through your code and ask “what happens if the
condition for the ‘if’ statement is false?”

> Is the code above bad? If so, what is the right way?

Yes, it's bad:

* The function name ‘f’ is completely unhelpful. Consider a reader of
the function who has no access to the inside of your head: Your
function should be named, preferably, as a verb phrase, to say what
the function *does* when it is called.

* The parameter name ‘c’ is completely undescriptive. Again, consider a
reader ignorant of your thinking: You should name parameters so they
help the reader know what the parameter is supposed to be and how it
will be interpreted.

* You're re-binding the parameter name ‘c’ to something different within
the function: it starts out bound to the input string, but by the time
the function ends you're expecting it to be bound to a datetime
object. Instead, you should be binding a *different* name to the
datetime object you create inside the function, and using that for the
return statement.

--
\ “Read not to contradict and confute, nor to believe and take |
`\ for granted … but to weigh and consider.” —Francis Bacon |
_o__) |
Ben Finney

Terry Reedy

unread,
Jun 19, 2009, 2:55:52 AM6/19/09
to pytho...@python.org
Matthew Wilson wrote:
> Here's the code that I'm feeding to pylint:
>
> $ cat f.py
> from datetime import datetime
>
> def f(c="today"):

pylint infers that you intend users to pass a string. Human would guess
the same at this point.

> if c == "today":
> c = datetime.today()

Now I guess that you actually intend c to be passed as a datetime
object. You only used the string as a type annotation, not as a real
default value. Something like 'record_date = None' is better.

> return c.date()

and here you ask for the input's date, which strings do not have.

Matthew Wilson

unread,
Jun 19, 2009, 9:36:01 AM6/19/09
to
On Fri 19 Jun 2009 02:55:52 AM EDT, Terry Reedy wrote:
>> if c == "today":
>> c = datetime.today()
>
> Now I guess that you actually intend c to be passed as a datetime
> object. You only used the string as a type annotation, not as a real
> default value. Something like 'record_date = None' is better.

Thanks for the feedback. I think I should have used a more obvious
string in my original example and a more descriptive parameter name.

So, pretend that instead of

c="today"

I wrote

record_date="defaults to today's date".

I know my way is unorthodox, but I think it is a little bit more obvious
to the reader than

record_date=None

The None is a signal to use a default value, but that is only apparent
after reading the code.

Thanks again for the comments.

Matt

Rhodri James

unread,
Jun 19, 2009, 9:53:57 AM6/19/09
to pytho...@python.org

It's a very common idiom, however, and in addition

record_data is None

is a cheap test, while

record_data == "defaults to today's date"

is an expensive test as well as easy to mistype.

--
Rhodri James *-* Wildebeest Herder to the Masses

mblume

unread,
Jun 19, 2009, 10:38:42 AM6/19/09
to

Think of what happens if somebody calls
some_result = f("yesterday")

HTH
Martin

Terry Reedy

unread,
Jun 19, 2009, 11:58:15 AM6/19/09
to pytho...@python.org
Matthew Wilson wrote:

> Thanks for the feedback. I think I should have used a more obvious
> string in my original example and a more descriptive parameter name.
>
> So, pretend that instead of
>
> c="today"
>
> I wrote
>
> record_date="defaults to today's date".
>
> I know my way is unorthodox,

Which is fine. Just do not by surprised when a fake default fakes out
pylint, which by its nature, must embody the orthodoxy of its author ;-)
To win general acceptance, that in turn must match the orthodoxy of the
general audience.

Message has been deleted

nn

unread,
Jun 19, 2009, 3:41:18 PM6/19/09
to

>>> 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'

nn

unread,
Jun 19, 2009, 3:52:37 PM6/19/09
to
On Jun 18, 8:56 pm, Matthew Wilson <m...@tplus1.com> wrote:

This is a limitation of static type checking and Pylint is a (limited)
static type checker for Python. A language with static type checking
would not have even allowed you to compile this. A static type
checkers sees this:

def midnight_next_day(initial_time [string or in python whatever gets
passed(T)]):

if initial_time [string (or T)]== [(some constant) string]:
initial_time [datetime] = datetime.now()
{implied else:
initial_time [string or (T)] }

return initial_time[could be either datetime or string (oops
string does not have date()) pylint doesn’t check for T].date() +
timedelta(days=1)

or this:

def midnight_next_day(initial_time [None object or (T)]):

if initial_time [None object or (T)]== [None object]:
initial_time [type datetime] = datetime.now()
{implied else:
initial_time [T] }

return initial_time[datetime (pylint doesn’t check for T)].date()
+ timedelta(days=1)

Aahz

unread,
Jun 20, 2009, 12:17:50 AM6/20/09
to
In article <87eitg2...@benfinney.id.au>,
Ben Finney <ben+p...@benfinney.id.au> wrote:

>Matthew Wilson <ma...@tplus1.com> writes:
>>
>> from datetime import datetime
>> def f(c="today"):
>> if c == "today":
>> c = datetime.today()
>> return c.date()
>
>* You're re-binding the parameter name ‘c’ to something different within
> the function: it starts out bound to the input string, but by the time
> the function ends you're expecting it to be bound to a datetime
> object. Instead, you should be binding a *different* name to the
> datetime object you create inside the function, and using that for the
> return statement.

Actually, I disagree that this is necessarily bad, although it's often a
yellow flag. The context in which it is possibly appropriate is when you
want your function to handle multiple types for one parameter and you
always cast the data to one single type.
--
Aahz (aa...@pythoncraft.com) <*> http://www.pythoncraft.com/

"as long as we like the same operating system, things are cool." --piranha

0 new messages