Date: Wed, 6 Aug 2014 16:21:31 -0700
From: "W. Trevor King" <wking@tremily.us>
To: David Bremner <bremner@debian.org>
Subject: Re: [PATCH v3] nmbug: Translate to Python
Message-ID: <20140806232131.GH31313@odin.tremily.us>
References:
 <84447a0ed48412e1587761d560d18cb5affd4f66.1405897133.git.wking@tremily.us>
	<878un36bd5.fsf@maritornes.cs.unb.ca>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
	protocol="application/pgp-signature"; boundary="ewQ5hdP4CtoTt3oD"
Content-Disposition: inline
In-Reply-To: <878un36bd5.fsf@maritornes.cs.unb.ca>
OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8;
	url=http://tremily.us/pubkey.txt
User-Agent: Mutt/1.5.23 (2014-03-12)
Cc: notmuch@notmuchmail.org
Precedence: list


--ewQ5hdP4CtoTt3oD
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Aug 04, 2014 at 09:14:46PM -0300, David Bremner wrote:
> W. Trevor King writes:
> > * Commands are no longer split into "most common", "other useful",
> >   and "less common" sets.  If we need something like this, I'd
> >   prefer workflow examples highlighting common commands in the
> >   module docstring (available with 'nmbug --help').
>=20
> I don't feel strongly about this, but I remember implementing it by
> request in the first version. OTOH, I think you shortened up the
> main help string when you split it.

I didn't intentionally remove any information.  I think it's just
shorter because command-specific details are now in the
command-specific docstring/help.

> We may want to think about a seperate man page as a follow project.

Works for me, but I think it would look a lot like the wiki page [1].

> One thing I did notice is that there is no hint to call nmbug
> {command} --help in the main docstring.

I'll add that in v4.

> > +#!/usr/bin/env python
> > +# Copyright (c) 2011 David Bremner
> > +# License: same as notmuch
>=20
> You should add your self, update the date, and probably explicitly
> state the license, as in Carl's patch for nmbug-status.

Will do in v4.

> > +__version__ =3D '0.2'
>=20
> Do we need/want a version distinct from that of notmuch?

nmbug is very loosely bound to the notmuch core.  To me it feels like
a separate project that happens to share the same version control
repository.  I'm happy to synchronize versions, but then we have to
remember to bump the nmbug version for each notmuch release.

> > +def _hex_quote(string, safe=3D'+@=3D:,'):
>=20
> I'm not sure I really understand what makes a function/variable
> "private" and hence prefixed with _ in your translation.

The public interface is what I thought was reasonably stable for folks
who want to call nmbug as a Python library.  It's basically the
command-line functionality, with a few other helpers that seemed
important enough to be worth preserving.  If we keep a separate nmbug
version, I'd cut major releases for anything that broke compatibility
on a public function.  I don't need to use nmbug as a library myself,
so this might all be over-engineered ;).

> > +    status, tree, stderr =3D _git(
>=20
> as a non-native speaker of python, I find this a bit hard to read.
> How about adding some parens to make the multiple return more clear,
> so
>=20
>     (status, tree, stderr) =3D _git(

That's legal, but I rarely see the parenthesized version in the wild.
For examples showing the unparenthesized version, see [2,3].
Parentheses are optional for Python tuples [4], so you'd only want
them if =E2=80=98=3D=E2=80=99 had a higher precedence than =E2=80=98,=E2=80=
=99.  That's my argument for
the unparenthesized version, but feel free to overrule me ;).

> I did notice that merging was noticably noisier than I remembered.=20
>=20
> > +    output =3D _collections.defaultdict(
> > +        lambda : _collections.defaultdict( # {tag: status_string}
> > +            lambda : ' '))  # default local status
>=20
> The initial comment is confusing (to me) because it looks like code.
> The two layers of defaultdict are a bit confusing too; it would help
> to mention the key of the outer dictionary.

In v4, I'll use a leading comment for the whole structure, instead of
interleaving the comments.  I'll also mention explicitly that the
outer layer is keyed by message id.

> I guess it makes sense to go for a relatively straight translation
> as a first step; I did wonder about whether using the python
> bindings to notmuch would speed things up.  Any ideas about how to
> even figure out where the bottlenecks are?

You could profile the Python script [5].  With my usual workflow, the
existing implementation isn't generating too many subprocesses.
Loading the database is probably slow though, so I'd expect reasonable
gains everywhere we call notmuch more than once.  I expect checkout
out working directories to also be slow, for the few command that do
that on behalf of Git.

Cheers,
Trevor


[1]: http://notmuchmail.org/nmbug/
[2]: https://docs.python.org/3.4/tutorial/introduction.html#first-steps-tow=
ards-programming
[3]: http://legacy.python.org/dev/peps/pep-3132/
[4]: https://docs.python.org/3.4/reference/expressions.html#parenthesized-f=
orms
[5]: https://docs.python.org/3.4/library/profile.html

--=20
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

--ewQ5hdP4CtoTt3oD
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJT4rh5AAoJEG8/JgBt8ol8Vy0P/2kCLd5L+IERhLY0GO8a9CQD
IazSnIzlMtYmeN97VM85zimoLnbhxnB7ATIGNk5B2tT43nB/t1Op2rd9ZqEzNExL
UcCokyTxDFvyfGtV1JqPA1hwgHcxr8ghGkA56NRxYCah9DixMd0zsmTbPaDfYuJq
S/if3LtByeTcD8Usgy9ZdU8A87ZR8Hrb+bOtWOr9OGFq38cNx7ymx9+OdNhi+KKU
ZHGasavcqxuGC5HEMkmJh0vpsOpJw+gVEuaNKnW29jG/vZnu5FVloH2YXfifdjDu
OLJ7bHhKZQ2uUGwdmX/2FViUSwW4/+j/YtkxKmJ9g2ekt5ro6PWuYnwVJlo00aZY
Vh2XnidzogjrpEkfZkeMyozIondW9tGLzx4ZJc52/jb0quMTdlIQFBuqb0vjnKp7
mhuifQZqabqOZIw6wuknThTWVOADqhgmb8oEEOMPEnG3V5fFljYAa8mMln8lIkpW
/5tCw3TeOlubk9EnEhRy0Y0hX9Qfdi9dPk6t63CMiT4hk7+No97WYEBK+CvPmby6
aZcSQr68E2zOYk8M6TPIv4PmyJc0r88M/BGPoj1hPw9B2wuKKzx3RW8JlvdjHJS/
0rG++DgCfHAXr4AxuwKy2ltjhiSJDlWC9054CNrnNhfdvl5N0rQJJ9PxtbJtJMFJ
xMzRS4BP2lBK6uemj19x
=DaTM
-----END PGP SIGNATURE-----

--ewQ5hdP4CtoTt3oD--