Re: [PATCH 3/3] python/notmuch2: provide binding for collect_tags()
Floris Bruynooghe venit, vidit, dixit 2021-01-11 21:33:16:
> Hi Micahel,
>
> Thanks for adding this feature!
>
> On Wed 06 Jan 2021 at 10:08 +0100, Michael J. Gruber wrote:
> > diff --git a/bindings/python-cffi/notmuch2/_query.py
> > b/bindings/python-cffi/notmuch2/_query.py
> > index 1db6ec96..a1310944 100644
> > --- a/bindings/python-cffi/notmuch2/_query.py
> > +++ b/bindings/python-cffi/notmuch2/_query.py
> > @@ -2,6 +2,7 @@ from notmuch2 import _base as base
> > from notmuch2 import _capi as capi
> > from notmuch2 import _errors as errors
> > from notmuch2 import _message as message
> > +from notmuch2 import _tags as tags
> > from notmuch2 import _thread as thread
> >
> >
> > @@ -66,6 +67,17 @@ class Query(base.NotmuchObject):
> > raise errors.NotmuchError(ret)
> > return count_p[0]
> >
> > +def collect_tags(self):
> > +"""Return the tags of messages matching this query."""
> > +msgs_pp = capi.ffi.new('notmuch_messages_t**')
> > +ret = capi.lib.notmuch_query_search_messages(self._query_p,
> > msgs_pp)
> > +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> > +raise errors.NotmuchError(ret)
> > +self._msgs_p = msgs_pp[0]
> > +tagset = tags.ImmutableTagSet(
> > +self, '_msgs_p', capi.lib.notmuch_messages_collect_tags)
> > +return tags.ImmutableTagSet._from_iterable(tagset)
> > +
> > def threads(self):
> > """Return an iterator over all the threads found by the query."""
> > threads_pp = capi.ffi.new('notmuch_threads_t **')
>
> How this this look on the C level for the memory allocator? If the
> query gets freed the messages also get freed? In that case indeed the
> messages do need to be kept alive together with the query indeed.
> Keeping them as an attribute on the query object seems reasonable I
> think, they'd be freed at the same time and I don't think this creates a
> circular reference.
I'm afraid I don't have the full picture of the memory model and object
lifetimes in the bindings.
> There's only a small detail though: to get the correct ObjectDestroyed
> exceptions you need Query._msgs_p to be a _base.MemoryPoiniter
> descriptor, like Query._query_p is currently. And then you also need to
> set it to None in Query._destroy.
I was trying to mimick what Query.messages() does: it creates msgs_pp
locally. The only reason for self._msgs_p in Thread.collect_tags() is
the way in which tags.ImmutableTagSet() expects the pointer: as an
attribute of self. But maybe there is a better way to call into
capi.lib.notmuch_messages_collect_tags()?
If not, and if I understand you correctly, then I should apply
diff --git i/bindings/python-cffi/notmuch2/_query.py
w/bindings/python-cffi/notmuch2/_query.py
index a1310944..fa58825d 100644
--- i/bindings/python-cffi/notmuch2/_query.py
+++ w/bindings/python-cffi/notmuch2/_query.py
@@ -17,6 +17,7 @@ class Query(base.NotmuchObject):
match libnotmuch's memory management.
"""
_query_p = base.MemoryPointer()
+_msgs_p = base.MemoryPointer()
def __init__(self, db, query_p):
self._db = db
@@ -40,6 +41,7 @@ class Query(base.NotmuchObject):
if self.alive:
capi.lib.notmuch_query_destroy(self._query_p)
self._query_p = None
+self._msgs_p = None
on top of this patch, right?
> It would also be really good to add at least one happy-path test for
> this, ensuring that you get the tags in the query. You could add some
> to TestQuery in test_database.py. The database is created with unread
> and inbox tags i think (see conftest.py:75) so the messages should
> already have some tags which is sufficient as you're not testing
> notmuch's behaviour itself but only that the bindings give you a right
> looking tagset (the tagset itself also has tests already, so no need to
> test this extensively, just see if you have some tags).
Thanks for the pointer. Will do for v2, comparing:
set(db.collect_tags('*')) == set(db.tags)
Michael
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH 3/3] python/notmuch2: provide binding for collect_tags()
Hi Micahel,
Thanks for adding this feature!
On Wed 06 Jan 2021 at 10:08 +0100, Michael J. Gruber wrote:
> diff --git a/bindings/python-cffi/notmuch2/_query.py
> b/bindings/python-cffi/notmuch2/_query.py
> index 1db6ec96..a1310944 100644
> --- a/bindings/python-cffi/notmuch2/_query.py
> +++ b/bindings/python-cffi/notmuch2/_query.py
> @@ -2,6 +2,7 @@ from notmuch2 import _base as base
> from notmuch2 import _capi as capi
> from notmuch2 import _errors as errors
> from notmuch2 import _message as message
> +from notmuch2 import _tags as tags
> from notmuch2 import _thread as thread
>
>
> @@ -66,6 +67,17 @@ class Query(base.NotmuchObject):
> raise errors.NotmuchError(ret)
> return count_p[0]
>
> +def collect_tags(self):
> +"""Return the tags of messages matching this query."""
> +msgs_pp = capi.ffi.new('notmuch_messages_t**')
> +ret = capi.lib.notmuch_query_search_messages(self._query_p, msgs_pp)
> +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +raise errors.NotmuchError(ret)
> +self._msgs_p = msgs_pp[0]
> +tagset = tags.ImmutableTagSet(
> +self, '_msgs_p', capi.lib.notmuch_messages_collect_tags)
> +return tags.ImmutableTagSet._from_iterable(tagset)
> +
> def threads(self):
> """Return an iterator over all the threads found by the query."""
> threads_pp = capi.ffi.new('notmuch_threads_t **')
How this this look on the C level for the memory allocator? If the
query gets freed the messages also get freed? In that case indeed the
messages do need to be kept alive together with the query indeed.
Keeping them as an attribute on the query object seems reasonable I
think, they'd be freed at the same time and I don't think this creates a
circular reference.
There's only a small detail though: to get the correct ObjectDestroyed
exceptions you need Query._msgs_p to be a _base.MemoryPoiniter
descriptor, like Query._query_p is currently. And then you also need to
set it to None in Query._destroy.
It would also be really good to add at least one happy-path test for
this, ensuring that you get the tags in the query. You could add some
to TestQuery in test_database.py. The database is created with unread
and inbox tags i think (see conftest.py:75) so the messages should
already have some tags which is sufficient as you're not testing
notmuch's behaviour itself but only that the bindings give you a right
looking tagset (the tagset itself also has tests already, so no need to
test this extensively, just see if you have some tags).
Otherwise LGTM!
Cheers,
Floris
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH 3/3] python/notmuch2: provide binding for collect_tags()
collect_tags() is accessible in the legacy bindings as a method on the
messages object returned by searches. In the cffi bindings, neither
queries nor their internal values are exposed, so provide a binding
analogous to count_messages().
Signed-off-by: Michael J Gruber
---
bindings/python-cffi/notmuch2/_database.py | 17 +
bindings/python-cffi/notmuch2/_query.py| 12
2 files changed, 29 insertions(+)
diff --git a/bindings/python-cffi/notmuch2/_database.py
b/bindings/python-cffi/notmuch2/_database.py
index 7592956a..f3452e11 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -618,6 +618,23 @@ class Database(base.NotmuchObject):
exclude_tags=exclude_tags)
return query.count_messages()
+def collect_tags(self, query, *,
+ omit_excluded=EXCLUDE.TRUE,
+ sort=SORT.UNSORTED, # Check this default
+ exclude_tags=None):
+"""Search the database for messages and collect tags.
+
+:returns: The tags of all messages found.
+:rtype: ImmutableTagset
+
+:raises ObjectDestroyedError: if used after destroyed.
+"""
+query = self._create_query(query,
+ omit_excluded=omit_excluded,
+ sort=sort,
+ exclude_tags=exclude_tags)
+return query.collect_tags()
+
def threads(self, query, *,
omit_excluded=EXCLUDE.TRUE,
sort=SORT.UNSORTED, # Check this default
diff --git a/bindings/python-cffi/notmuch2/_query.py
b/bindings/python-cffi/notmuch2/_query.py
index 1db6ec96..a1310944 100644
--- a/bindings/python-cffi/notmuch2/_query.py
+++ b/bindings/python-cffi/notmuch2/_query.py
@@ -2,6 +2,7 @@ from notmuch2 import _base as base
from notmuch2 import _capi as capi
from notmuch2 import _errors as errors
from notmuch2 import _message as message
+from notmuch2 import _tags as tags
from notmuch2 import _thread as thread
@@ -66,6 +67,17 @@ class Query(base.NotmuchObject):
raise errors.NotmuchError(ret)
return count_p[0]
+def collect_tags(self):
+"""Return the tags of messages matching this query."""
+msgs_pp = capi.ffi.new('notmuch_messages_t**')
+ret = capi.lib.notmuch_query_search_messages(self._query_p, msgs_pp)
+if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
+raise errors.NotmuchError(ret)
+self._msgs_p = msgs_pp[0]
+tagset = tags.ImmutableTagSet(
+self, '_msgs_p', capi.lib.notmuch_messages_collect_tags)
+return tags.ImmutableTagSet._from_iterable(tagset)
+
def threads(self):
"""Return an iterator over all the threads found by the query."""
threads_pp = capi.ffi.new('notmuch_threads_t **')
--
2.30.0.rc0.297.gbcca948854
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
