Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-05-02 Thread David Bremner
Ruben Pollan  writes:

> Message.get_property (prop) returns a string with the value of the property 
> and
> Message.get_properties (prop, exact=False) yields key, value pairs

pushed this version to master.  Thanks for your contribution.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-05-02 Thread meskio
Quoting David Bremner (2018-05-02 02:08:26)
> Ruben Pollan  writes:
> 
> > Message.get_property (prop) returns a string with the value of the property 
> > and
> > Message.get_properties (prop, exact=False) yields key, value pairs
> > ---
> >  bindings/python/docs/source/message.rst |  4 ++
> >  bindings/python/notmuch/globals.py  |  5 +++
> >  bindings/python/notmuch/message.py  | 80 
> > -
> >  3 files changed, 88 insertions(+), 1 deletion(-)
> >
> 
> This version passes the first test (after fixing the format, as you
> noted), but it looks like get_properties is returning pairs of
> bytestrings.
> 
> FAIL   [15] msg.get_properties (python)
> --- T610-message-property.16.EXPECTED   2018-05-02 00:02:11.160028179 
> +
> +++ T610-message-property.16.OUTPUT 2018-05-02 00:02:11.164028171 
> +
> @@ -1,4 +1,4 @@
> -testkey1 = alice
> -testkey1 = bob
> -testkey1 = testvalue1
> -testkey1 = testvalue2
> +b'testkey1' = b'alice'
> +b'testkey1' = b'bob'
> +b'testkey1' = b'testvalue1'
> +b'testkey1' = b'testvalue2'
> 
> I don't _think_ that's what we want. We had some discussion before and
> decided that it was reasonable to only support utf-8 properties, so
> converting to strings should be OK?

I added the 'decode("utf-8")' to get_property but I didn't to get_properties.  
Next patch fixes it.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-05-01 Thread David Bremner
Ruben Pollan  writes:

> Message.get_property (prop) returns a string with the value of the property 
> and
> Message.get_properties (prop, exact=False) yields key, value pairs
> ---
>  bindings/python/docs/source/message.rst |  4 ++
>  bindings/python/notmuch/globals.py  |  5 +++
>  bindings/python/notmuch/message.py  | 80 
> -
>  3 files changed, 88 insertions(+), 1 deletion(-)
>

This version passes the first test (after fixing the format, as you
noted), but it looks like get_properties is returning pairs of
bytestrings.

FAIL   [15] msg.get_properties (python)
--- T610-message-property.16.EXPECTED   2018-05-02 00:02:11.160028179 
+
+++ T610-message-property.16.OUTPUT 2018-05-02 00:02:11.164028171 
+
@@ -1,4 +1,4 @@
-testkey1 = alice
-testkey1 = bob
-testkey1 = testvalue1
-testkey1 = testvalue2
+b'testkey1' = b'alice'
+b'testkey1' = b'bob'
+b'testkey1' = b'testvalue1'
+b'testkey1' = b'testvalue2'

I don't _think_ that's what we want. We had some discussion before and
decided that it was reasonable to only support utf-8 properties, so
converting to strings should be OK?

here's the proposed tests. 

diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 74b3f5a1..c903b2b6 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -256,4 +256,34 @@ id:4efc743a.3060...@april.org
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "msg.get_property (python)"
+test_python <<'EOF'
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
+msg = db.find_message("4efc743a.3060...@april.org")
+print("testkey1 = {0}".format(msg.get_property("testkey1")))
+print("testkey3 = {0}".format(msg.get_property("testkey3")))
+EOF
+cat <<'EOF' > EXPECTED
+testkey1 = alice
+testkey3 = alice3
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "msg.get_properties (python)"
+test_python <<'EOF'
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
+msg = db.find_message("4efc743a.3060...@april.org")
+for (key,val) in msg.get_properties("testkey1"):
+print("{0} = {1}".format(key,val))
+EOF
+cat <<'EOF' > EXPECTED
+testkey1 = alice
+testkey1 = bob
+testkey1 = testvalue1
+testkey1 = testvalue2
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-05-01 Thread meskio
Quoting David Bremner (2018-05-01 15:06:38)
> running the test below, I get
> 
> Traceback (most recent call last):
>   File "", line 4, in 
>   File 
> "/home/bremner/software/upstream/notmuch/bindings/python/notmuch/message.py", 
> line 480, in get_property
> value = c_char_p("")
> TypeError: bytes or integer address expected instead of str instance

Ups, I was using python2 to test it. I see it doesn't work with python three.  
I'll send an update.

> diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
> index 74b3f5a1..f7220565 100755
> --- a/test/T610-message-property.sh
> +++ b/test/T610-message-property.sh
> @@ -89,6 +89,18 @@ testkey2 = NULL
>  EOF
>  test_expect_equal_file EXPECTED OUTPUT
>  
> +test_begin_subtest "msg.get_property (python)"
> +test_python <<'EOF'
> +import notmuch
> +db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
> +msg = db.find_message("4efc743a.3060...@april.org")
> +print("testkey1[1] = %s\n".format(msg.get_property("testkey1")))

I think this should be (notice the {0} instead of %s):
print("testkey1[1] = {0}".format(msg.get_property("testkey1")))


-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-05-01 Thread David Bremner
Ruben Pollan  writes:

> Message.get_property (prop) returns a string with the value of the property 
> and
> Message.get_properties (prop, exact=False) yields key, value pairs
> ---
>  bindings/python/docs/source/message.rst |  4 ++
>  bindings/python/notmuch/globals.py  |  5 +++
>  bindings/python/notmuch/message.py  | 80 
> -
>  3 files changed, 88 insertions(+), 1 deletion(-)

I started to write some simple tests for this, but I didn't get very
far.

running the test below, I get

Traceback (most recent call last):
  File "", line 4, in 
  File 
"/home/bremner/software/upstream/notmuch/bindings/python/notmuch/message.py", 
line 480, in get_property
value = c_char_p("")
TypeError: bytes or integer address expected instead of str instance


diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 74b3f5a1..f7220565 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -89,6 +89,18 @@ testkey2 = NULL
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "msg.get_property (python)"
+test_python <<'EOF'
+import notmuch
+db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
+msg = db.find_message("4efc743a.3060...@april.org")
+print("testkey1[1] = %s\n".format(msg.get_property("testkey1")))
+EOF
+cat <<'EOF' > EXPECTED
+testkey1[1] = testvalue1
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "notmuch_message_remove_all_properties"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 EXPECT0(notmuch_message_remove_all_properties (message, NULL));
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2018-04-27 Thread meskio
Quoting David Bremner (2017-12-23 16:59:51)
> Ruben Pollan  writes:
> > +
> > +def get_properties(self, prop="", exact=False):
> 
> As far as I understand, you also need to update docs/source/message.rst
> so that your new methods are documented in the sphinx docs.
> 
> > +""" Get the properties for *message*, returning
> > +notmuch_message_properties_t object which can be used to iterate
> > +over all properties.
> 
> This seeems to be wrong (or at last confusing) for the python bindings.
> 
> > +
> > +:param prop: The name of the property to get. Otherwise it will 
> > return
> > + the full list of properties of the message.
> > +:param exact: if True, require exact match with key. Otherwise
> > +  treat as prefix.
> > +:returns: A dictionary with the property names and values {key: 
> > value}
> > +:raises: :exc:`NotInitializedError` if message has not been
> > + initialized
> > +"""
> > +if not self._msg:
> > +raise NotInitializedError()
> > +
> > +properties_dict = {}
> > +properties = Message._get_properties(self._msg, prop, exact)
> 
> Now that the database.get_configs method is merged, I'd prefer to be 
> consistent
> with that, and define a generator that yields key/value pairs. It's easy
> enough for someone to use a dictionary comprehension to get a dict from
> that if they want it.  Sorry to be making extra work for you.

Good to me, I'm sending an update with all this fixed.

Sorry for letting it hung for so long.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-12-23 Thread David Bremner
Ruben Pollan  writes:
> +
> +def get_properties(self, prop="", exact=False):

As far as I understand, you also need to update docs/source/message.rst
so that your new methods are documented in the sphinx docs.

> +""" Get the properties for *message*, returning
> +notmuch_message_properties_t object which can be used to iterate
> +over all properties.

This seeems to be wrong (or at last confusing) for the python bindings.

> +
> +:param prop: The name of the property to get. Otherwise it will 
> return
> + the full list of properties of the message.
> +:param exact: if True, require exact match with key. Otherwise
> +  treat as prefix.
> +:returns: A dictionary with the property names and values {key: 
> value}
> +:raises: :exc:`NotInitializedError` if message has not been
> + initialized
> +"""
> +if not self._msg:
> +raise NotInitializedError()
> +
> +properties_dict = {}
> +properties = Message._get_properties(self._msg, prop, exact)

Now that the database.get_configs method is merged, I'd prefer to be consistent
with that, and define a generator that yields key/value pairs. It's easy
enough for someone to use a dictionary comprehension to get a dict from
that if they want it.  Sorry to be making extra work for you.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-30 Thread Daniel Kahn Gillmor
On Thu 2017-11-30 09:44:19 -0400, David Bremner wrote:
> Floris Bruynooghe  writes:
>
>> This also has the binary question problem, is this returned as bytes or
>> as str?  Current Python bindings seem to go for .decode('utf-8',
>> errors='ignore') afaik which is somewhat lossy.
>>
>
> IMHO there's no reason to support non-utf8 properties. We should
> document this restriction now while adoption is relatively light.

Agreed, my reading of the source is that we expect strings for both
property names and property values.  If they're both strings, we should
not support anything other than UTF-8 in either case.

  --dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-30 Thread David Bremner
Floris Bruynooghe  writes:

> Daniel Kahn Gillmor  writes:
>
> This also has the binary question problem, is this returned as bytes or
> as str?  Current Python bindings seem to go for .decode('utf-8',
> errors='ignore') afaik which is somewhat lossy.
>

IMHO there's no reason to support non-utf8 properties. We should
document this restriction now while adoption is relatively light.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-29 Thread Floris Bruynooghe
Daniel Kahn Gillmor  writes:

> On Tue 2017-11-28 23:46:11 +0100, Ruben Pollan wrote:
>> Message.get_property (prop) returns a string with the value of the property 
>> and
>> Message.get_properties (prop, exact=False) returns a list [(key, value)]
>
> This looks like a sensible approach to me.  I'd be curious to hear what
> others think of this.
>
> In considering the API design space here, it occurs to me that it might
> be more pythonic for get_properties to return a dict like:

I would probably model properties as a dictionary in notdb, making this
a collections.abc.MutableMapping implementation with a .get_all(prop)
method inspired from the stdlib email.message package.  This kind of
also implies making properties access a property rather then a method
call:

msg.properties['prop'] = 'foo'
msg.properties['prop'] = 'bar'
msg.properties['prop'] == 'foo'  # pot luck
msg.properties.get_all('prop') == {'foo', 'bar'}  # properties are unsorted are 
they?

This also has the binary question problem, is this returned as bytes or
as str?  Current Python bindings seem to go for .decode('utf-8',
errors='ignore') afaik which is somewhat lossy.


Cheers,
Floris
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-29 Thread meskio
Quoting Daniel Kahn Gillmor (2017-11-29 02:57:24)
> On Tue 2017-11-28 23:46:11 +0100, Ruben Pollan wrote:
> > Message.get_property (prop) returns a string with the value of the property 
> > and
> > Message.get_properties (prop, exact=False) returns a list [(key, value)]
> 
> This looks like a sensible approach to me.  I'd be curious to hear what
> others think of this.
> 
> In considering the API design space here, it occurs to me that it might
> be more pythonic for get_properties to return a dict like:
> 
>{ key: [ value, … ], key: [ value, … ] }
> 
> Any reason you chose one over the other?  My python-fu is shallow, so
> please don't take my aesthetic guesswork as authoritative; but i'm
> imagining a user wanting to grab a bunch of properties and then easily
> access them by key, and the dict seems like the simple way to do that.

Yes, the dict is more pythonic. I thought about it, I went for the tuples it 
was 
simpler to implement (and use in my use case). But giving a second thought it 
makes more sense to do a dict.

> Also, does get_properties() work with prop=None to fetch all properties?
> if so, maybe that should be the default?

I didn't thought about that, but you are right, with prop="" you get the full 
list of properties of the message. Nice, let's put it as default value.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-28 Thread Daniel Kahn Gillmor
On Tue 2017-11-28 23:46:11 +0100, Ruben Pollan wrote:
> Message.get_property (prop) returns a string with the value of the property 
> and
> Message.get_properties (prop, exact=False) returns a list [(key, value)]

This looks like a sensible approach to me.  I'd be curious to hear what
others think of this.

In considering the API design space here, it occurs to me that it might
be more pythonic for get_properties to return a dict like:

   { key: [ value, … ], key: [ value, … ] }

Any reason you chose one over the other?  My python-fu is shallow, so
please don't take my aesthetic guesswork as authoritative; but i'm
imagining a user wanting to grab a bunch of properties and then easily
access them by key, and the dict seems like the simple way to do that.

Also, does get_properties() work with prop=None to fetch all properties?
if so, maybe that should be the default?

To be clear, I'd be fine with a response that disagrees with these
suggestions (especially if it explains why) and then adopting this
patch; i just want to make sure they've been considered before we lock
in the API.

Thanks for working on this, Meskio!

   --dkg


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)

2017-11-28 Thread meskio
Quoting Daniel Kahn Gillmor (2017-11-17 10:03:09)
> On Wed 2017-11-15 23:29:54 +0100, Ruben Pollan wrote:
> > Message.get_property (prop) returns a string with the value of the property.
> 
> Upon review, this is actually insufficient for making robust use of the
> session-key series :(
> 
> In particular, it only returns the first value for the session key
> returned.

See the last patch. I added the implementation of get_properties (and I use it 
in my session-key branch of alot). Hopefully this solves the problem.

> In the session-key series, i work around this by simply trying each
> session key against an encrypted part until i find one that works. It
> would be "cleaner" (more principled) to somehow associate each session
> key with the part(s) of the message file(s) that it is capable of
> decrypting, but that's a lot of bookkeeping -- i think it's actually
> "cleaner" (less code, less computation in the standard case) to just
> take the current approach.

Fair enough for me, I'm copying this approach in alot as well.

-- 
meskio | http://meskio.net/
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 My contact info: http://meskio.net/crypto.txt
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Nos vamos a Croatan.


signature.asc
Description: signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch