Re: [PATCH] python: add bindings for notmuch_message_get_propert(y/ies)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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