[PATCH] python/thread: always return a string in get_subject/authors

[PATCH] python/thread: always return a string in get_subject/authors

[Anton Khirnov]

Now None is returned when those don't exist, which is inconvenient to
deal with.
---
 bindings/python/notmuch/thread.py |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/bindings/python/notmuch/thread.py b/bindings/python/notmuch/thread.py
index eebd6cb..cf26957 100644
--- a/bindings/python/notmuch/thread.py
+++ b/bindings/python/notmuch/thread.py
@@ -281,7 +281,8 @@ class Thread(object):
         """
         if self._thread is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
-        return Thread._get_authors(self._thread)
+        ret = Thread._get_authors(self._thread)
+        return ret if ret else ''
 
     def get_subject(self):
         """Returns the Subject of 'thread'
@@ -291,7 +292,8 @@ class Thread(object):
         """
         if self._thread is None:
             raise NotmuchError(STATUS.NOT_INITIALIZED)
-        return Thread._get_subject(self._thread)
+        ret = Thread._get_subject(self._thread)
+        return ret if ret else ''
 
     def get_newest_date(self):
         """Returns time_t of the newest message date
-- 
1.7.4.4

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Austin Clements]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

Your commit message is inconsistent with your change; is your intent to
return None or the empty string?  Also, could you modify your commit message
to say what "those" are?
On May 9, 2011 3:06 AM, "Anton Khirnov" <anton@khirnov.net> wrote:

[-- Attachment #2: Type: text/html, Size: 454 bytes --]

Re: [PATCH] python/thread: always return a string in get_subject/authors

[David Bremner]

On Mon,  9 May 2011 09:06:34 +0200, Anton Khirnov <anton@khirnov.net> wrote:
> Now None is returned when those don't exist, which is inconvenient to
> deal with.

I'm not using the python bindings, but from a philosophical point of
view, this change makes me a bit uncomfortable since it apparently
merges two cases together, and makes an error (no Subject)
indistinguishable from an odd situation (Subject of empty string).
Or am I missing something here?

All the best,

David

Re: [PATCH] python/thread: always return a string in get_subject/authors

[servilio]

On 9 May 2011 08:20, David Bremner <david@tethera.net> wrote:
> On Mon,  9 May 2011 09:06:34 +0200, Anton Khirnov <anton@khirnov.net> wrote:
>> Now None is returned when those don't exist, which is inconvenient to
>> deal with.
>
> I'm not using the python bindings, but from a philosophical point of
> view, this change makes me a bit uncomfortable since it apparently
> merges two cases together, and makes an error (no Subject)
> indistinguishable from an odd situation (Subject of empty string).
> Or am I missing something here?

I see the the same issue.

Servilio

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Anton Khirnov]

On Mon, 09 May 2011 09:20:41 -0300, David Bremner <david@tethera.net> wrote:
> On Mon,  9 May 2011 09:06:34 +0200, Anton Khirnov <anton@khirnov.net> wrote:
> > Now None is returned when those don't exist, which is inconvenient to
> > deal with.
> 
> I'm not using the python bindings, but from a philosophical point of
> view, this change makes me a bit uncomfortable since it apparently
> merges two cases together, and makes an error (no Subject)
> indistinguishable from an odd situation (Subject of empty string).
> Or am I missing something here?

The question is whether this is really a problem.

For a single message, it might make sense to distinguish between 'no
header' and 'empty header'.

But those aren't message headers, those are thread properties. And I'd
argue that a thread always has authors and a subject (possibly empty).

--
Anton Khirnov

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Jesse Rosenthal]

On Mon, 09 May 2011 17:13:10 +0200, Anton Khirnov <anton@khirnov.net> wrote:
> But those aren't message headers, those are thread properties. And I'd
> argue that a thread always has authors and a subject (possibly empty).

The RFC says yes on the author, no on the subject. The only things
guaranteed are "From:" and originating timestamp. So I'm not sure why
subject should be guaranteed a string result and not, say, "Cc." 

My sense is that Python users are prety good with testing against None,
especially since (not "") == (not []) == (not None) == True. This change
seems like it would end up producing more inconsistencies with the way
you deal with headers, by producing special cases.

--Jesse

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Jesse Rosenthal]

On Mon, 09 May 2011 11:23:16 -0400, Jesse Rosenthal <jrosenthal@jhu.edu> wrote:
> The RFC says yes on the author, no on the subject. The only things
> guaranteed are "From:" and originating timestamp. So I'm not sure why
> subject should be guaranteed a string result and not, say, "Cc." 

Apologies -- I realize now you were talking about threads and not
messages, so I can't defer to RFCs. I still agree with the others, and
about how python users would deal with it. But I see your point better
now. Sorry to clutter.

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

On Mon, 09 May 2011 09:20:41 -0300, David Bremner <david@tethera.net> wrote:
> On Mon,  9 May 2011 09:06:34 +0200, Anton Khirnov <anton@khirnov.net> wrote:
> > Now None is returned when those don't exist, which is inconvenient to
> > deal with.
> 
> I'm not using the python bindings, but from a philosophical point of
> view, this change makes me a bit uncomfortable since it apparently
> merges two cases together, and makes an error (no Subject)
> indistinguishable from an odd situation (Subject of empty string).
> Or am I missing something here?

Hi there,

This change makes me a bit uncomfortable too. 3 Reasons:

- I believe users should be able to distinguish the case when someone
  uses an empty subject, and when someone doesn't specify a subject at
  all.

- People have been writing code and breaking backwards compatability for
  such a small gain doesn't really seem worth it.

- Testing-wise this is easy. Just test for "if subject:" on the returned
  value and you'll get both cases (empty and non-existing).

But if people really want it, I won't object.

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] python/thread: always return a string in get_subject/authors

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On 05/09/2011 09:00 PM, Sebastian Spaeth wrote:
> On Mon, 09 May 2011 09:20:41 -0300, David Bremner <david@tethera.net> wrote:
>> On Mon,  9 May 2011 09:06:34 +0200, Anton Khirnov <anton@khirnov.net> wrote:
>>> Now None is returned when those don't exist, which is inconvenient to
>>> deal with.
>>
>> I'm not using the python bindings, but from a philosophical point of
>> view, this change makes me a bit uncomfortable since it apparently
>> merges two cases together, and makes an error (no Subject)
>> indistinguishable from an odd situation (Subject of empty string).
>> Or am I missing something here?
> 
> This change makes me a bit uncomfortable too. 3 Reasons:
> 
> - I believe users should be able to distinguish the case when someone
>   uses an empty subject, and when someone doesn't specify a subject at
>   all.

I'm going to "me too!" this sentiment as well.  Please do *not* conflate
no-subject with subject-is-empty-string.

If we leave them distinct, the caller is free to conflate them if they
want.  But if we conflate the two states first, there's no way for the
caller to differentiate between the two if they want to.

Thanks,

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]