What's the tidy/elegant way to protect this against null/empty parameters?

[tin...@isbd.co.uk]

I want to fix an error in some code I have installed, however I don't
really want to just bodge it.

The function producing the error is:-

def get_text(self, idx): # override !
node = self.items[idx]

a= [
", ".join(node.tags),
node.comment,
node.folderName,
cd2rd(node.date),
node.name,
'[' + self.rating_stars[node.rating] + ']'
] [self.select]

return a


The error occurs when node[] (or at least its members) turn out to be
empty, you get a Traceback that ends with:-

File "/usr/lib/jbrout/jbrout/listview.py", line 608, in draw_cell layout.set_text(self.get_text(thumbnail_num))
File "/usr/lib/jbrout/jbrout.py", line 325, in get_text ", ".join(node.tags),
TypeError: sequence item 0: expected string, NoneType found

Now its *probably* something higher up the tree causing the problem
(it's only one particular image in 20 thousand or so that breaks
things) but I really want to just get things working. So, what's the
neatest way to protect the get_text() method from empty data?


--
Chris Green

[Paul Rubin]

tin...@isbd.co.uk writes:
> I want to fix an error in some code I have installed, however I don't

> really want to just bodge it. ...

> Now its *probably* something higher up the tree causing the problem
> (it's only one particular image in 20 thousand or so that breaks
> things) but I really want to just get things working.

That means you do really want to just bodge it, doesn't it? (I'm
reading "bodge" as something like "kludge" but maybe it means something
different.)

> So, what's the neatest way to protect the get_text() method from empty
> data?

I'd say the use of None as a picture is already a code smell: figure out
where it is coming from, and fix it. Or, if you want to just bodge it
(by checking for None and returning the empty string or something)),
then try it and see if it helps. I'm assuming this is something
noncritical that you're running on your own computer for convenience. I
wouldn't ship code to a customer without a more careful fix.

[Chris Rebert]

On Mon, Oct 15, 2012 at 4:23 AM, <tin...@isbd.co.uk> wrote:
> I want to fix an error in some code I have installed, however I don't
> really want to just bodge it.

"bodge". Well, I learned a new word this morning!

> The function producing the error is:-
>
> def get_text(self, idx): # override !
> node = self.items[idx]
>
> a= [
> ", ".join(node.tags),
> node.comment,
> node.folderName,
> cd2rd(node.date),
> node.name,
> '[' + self.rating_stars[node.rating] + ']'
> ] [self.select]
>
> return a
>
>
> The error occurs when node[] (or at least its members) turn out to be
> empty,

To be precise: when node.tags contains one or more `None`s (Python's
equivalent of what other languages call "null" or "nil").
That's what the traceback is saying.

> you get a Traceback that ends with:-
>
> File "/usr/lib/jbrout/jbrout/listview.py", line 608, in draw_cell layout.set_text(self.get_text(thumbnail_num))

Ah, so this is apparently regarding https://code.google.com/p/jbrout/
. Would have been nice not to have had to search and then only locate
it indirectly. Something to consider next time you write in...
Make sure you report your bug upstream!

> File "/usr/lib/jbrout/jbrout.py", line 325, in get_text ", ".join(node.tags),
> TypeError: sequence item 0: expected string, NoneType found
>
> Now its *probably* something higher up the tree causing the problem
> (it's only one particular image in 20 thousand or so that breaks
> things) but I really want to just get things working. So, what's the
> neatest way to protect the get_text() method from empty data?

Filter out the `None`s with a generator expression:
", ".join(tag for tag in node.tags if tag is not None),

Cheers,
Chris

[Roy Smith]

In article <1b8tk9-...@chris.zbmc.eu>, tin...@isbd.co.uk wrote:

> The function producing the error is:-
>
> def get_text(self, idx): # override !
> node = self.items[idx]
>
> a= [
> ", ".join(node.tags),
> node.comment,
> node.folderName,
> cd2rd(node.date),
> node.name,
> '[' + self.rating_stars[node.rating] + ']'
> ] [self.select]
>
> return a
>
>
> The error occurs when node[] (or at least its members) turn out to be
> empty, you get a Traceback that ends with:-
>
> File "/usr/lib/jbrout/jbrout/listview.py", line 608, in draw_cell
> layout.set_text(self.get_text(thumbnail_num))
> File "/usr/lib/jbrout/jbrout.py", line 325, in get_text ",
> ".join(node.tags),
> TypeError: sequence item 0: expected string, NoneType found
>
> Now its *probably* something higher up the tree causing the problem
> (it's only one particular image in 20 thousand or so that breaks
> things) but I really want to just get things working. So, what's the
> neatest way to protect the get_text() method from empty data?

Well, you don't describe what get_text() is supposed to return First,
you build a list of what I'm guessing are all strings, then you index
into it and return one of the values. So, I'm guessing get_text() is
supposed to return a string.

At a low level, you can certainly fix that by testing to see if
self.items[idx] returns what you're expecting (an instance of Node?) and
returning an empty string if it's not:

> def get_text(self, idx): # override !
> node = self.items[idx]

> if not node:
> return ""

>
> a= [
> ", ".join(node.tags),
> node.comment,
> node.folderName,
> cd2rd(node.date),
> node.name,
> '[' + self.rating_stars[node.rating] + ']'
> ] [self.select]
>
> return a

Whether that makes sense in your program, I have no idea. What does it
mean for node to be empty? Is this a normal occurrence? If so, then
your code needs to deal with properly (the suggest above being just one
possible way).

Or, is it "impossible" for node to be empty? In that case, that fact
that it *is* empty is a bug, and the above suggestion will just hide the
bug for one more level and make it that much harder to figure out what's
really going on. What you might really want to do is sprinkle your code
with "assert node" statements. This will force your program to crash
and burn the first time node is empty, which might help you figure out
why it is.

[Miki Tebeka]

> I want to fix an error in some code I have installed, ...
Apart from all the reasons why it's bad (see the Python Zen #10). One way to do it is:
return [i or '' for i in a]

[Terry Reedy]

On 10/15/2012 7:23 AM, tin...@isbd.co.uk wrote:
> I want to fix an error in some code I have installed, however I don't
> really want to just bodge it.
>
> The function producing the error is:-
>
> def get_text(self, idx): # override !
> node = self.items[idx]
>
> a= [
> ", ".join(node.tags),
> node.comment,
> node.folderName,
> cd2rd(node.date),
> node.name,
> '[' + self.rating_stars[node.rating] + ']'
> ] [self.select]
>
> return a
>
>
> The error occurs when node[] (or at least its members) turn out to be
> empty,

This is not the problem.

> you get a Traceback that ends with:-
>
> File "/usr/lib/jbrout/jbrout/listview.py", line 608, in draw_cell layout.set_text(self.get_text(thumbnail_num))
> File "/usr/lib/jbrout/jbrout.py", line 325, in get_text ", ".join(node.tags),
> TypeError: sequence item 0: expected string, NoneType found

The specific problem is that node.tags is supposed to be a sequence of
strings and somehow one instead has None as the first, and probably last
item. This was likely intended to indicate an empty list, but the way to
do that is to have a empty list, which would have worked just fine. In
other words, the likely problem is that node.tags is *not* an empty
sequence when it should be.

>>> ','.join([None])
Traceback (most recent call last):
File "<pyshell#9>", line 1, in <module>
','.join([None])
TypeError: sequence item 0: expected str instance, NoneType found
>>> ','.join([])
''

--
Terry Jan Reedy

[Jean-Michel Pichavant]

> --
> http://mail.python.org/mailman/listinfo/python-list

Hi Chris,

Usually, you want to keep your variable types consistent. For instance, if node.tags is supposed to be a string, then make it so that it is always a string.
However NoneType may be one inconsistent type that is acceptable for any variable. It depends on the situation, and your personal preferences.

To go back to your example, None may have a different meaning that an empty string. Here's a way to redefine locally a None object into an empty string:

", ".join(node.tags or '')

if node itself can be None, here's a way to protect against None attribute access:

", ".join(node and node.tags)

you can combine the 2 methods:

", ".join((node and node.tags) or '')


This method uses the fact that:
A and B returns B if bool(A) and bool(B) are True, returns the first non True between A and B.
A or B returns B if bool(A) is False, returns A if bool(A) is True.

JM

[Marco Nawijn]

Hi,

Instead of protecting against empty data, you could just catch the exception, issue a warning and return a default "error" node which is valid. So something like (not tested):

def get_text(self, idx): # override !
node = self.items[idx]

error_a = "A valid, but erroneous representation of a"

try:

a= [
", ".join(node.tags),
node.comment,
node.folderName,
cd2rd(node.date),
node.name,
'[' + self.rating_stars[node.rating] + ']'
] [self.select]

except TypeError:
print 'Oops, something went wrong'
a = error_a
# You should always have a valid a here (or another exception has occured)
return a

[tin...@isbd.co.uk]

That sounds like a reasonable approach, thank you (and all the other ideas).

--
Chris Green