[PING] updated benchmark display

[Carl Friedrich Bolz]

John Arbash Meinel wrote:
[snip]
>> Could someone please look at this branch again? We think it is close to
>> merging and Martin said on IRC that he liked the output. It is
>> registered at launchpad as bench-display. I also attached the diff.
>>
>> Cheers,
>>
>> Carl Friedrich
>>
>
> There are quite a few PEP8 violations, but I could clean those up.

Just as a sidenode: did somebody ever check out pychecker or pylint
and tried to get them to complain about all PEP8 violation?

> I'm a little more concerned about how it was implemented, though.
>
> It seems that most things are in plain tools/*. Which is okay, though if
> they really are an aggregate, then they probably should be in their own
> subdir.

Ok, that part would be easy.

> And then you have a test that runs, and expects to be able to import
> 'tools'. Which will break if bzr is installed (because it doesn't
> install the 'tools' directory).

I see. I guess when bzr is installed it would not be very important to
be able to use the benchmark display anyway, since it is more of a
developer tool.

> I'm also concerned that you are importing 'tools/benchmark_report' which
> is importing Image, ImageDraw, etc.
>
> I assume these come from PIL. However, I don't know that we want an
> explicit dependency on PIL for bzr. Which would be a good reason why the
> benchmark display stuff should be a plugin.
>
> It can still provide test cases because we can use:
> bzrlib.plugins.<plugin_name>.test_suite()
>
> To get any extra tests that you would want to run.
>
> I really don't want to merge stuff into the core that adds dependencies
> on stuff that isn't needed for standard operation.

Ok, I think there are several ways to go there. One is to make it a
plugin (which I think we would like to avoid, but of course do if
necessary). Another one is to make the tests skip properly when PIL is not
present and solve the problem of not being able to import tools in the
installed version.

The third one is to remove the PIL dependency by and using HTML images
instead, which would make the images look a bit less good, and be a bit
of effort. On the other hand, it would be easier to make parts of the
images "hover" if you put the mouse over it.

> I also wonder why you need to use 'pyxml' at all, rather than using
> cElementTree. If it makes things easier for you, that is fine. But
> cElementTree represents documents as a list of elements. It seems to be
> a pretty good (and quite fast) api. And it seems to make more sense to
> re-use the one we already have, rather than adding another one.

We mostly used pyxml because we are very familiar with it and think that
the API is extremely nice for html generation. But if that's the only
remaining problem we could probably replace it by cElementTree, if
absolutely necessary. Pyxml is not really a huge burden after all (just
a small extra file).

>
> So the #1 thing in my mind, is that this should probably be a plugin.
> The #2 is that it needs some PEP8 cleanup. And the #3 is at least an
> explanation why we want to bring in extra dependencies.
>
> If we decide not to do it as a plugin, then we need to add a lot of
> checking, so that the test suite doesn't fail if someone doesn't have
> PIL installed.
>
> As it stands, I don't think they could even run the test suite, because
> they will get an ImportError while it tries to load all the possible
> test cases.

Cheers,

Carl Friedrich