pcp
[Top] [All Lists]

Re: [pcp] pmrep: add xlsx support

To: myllynen@xxxxxxxxxx
Subject: Re: [pcp] pmrep: add xlsx support
From: Nathan Scott <nathans@xxxxxxxxxx>
Date: Tue, 15 Dec 2015 16:54:53 -0500 (EST)
Cc: pcp developers <pcp@xxxxxxxxxxx>
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <5670184B.7000603@xxxxxxxxxx>
References: <5670184B.7000603@xxxxxxxxxx>
Reply-to: Nathan Scott <nathans@xxxxxxxxxx>
Thread-index: jk8yDL3K/glOk7TMWBtrq3NsPNmfxA==
Thread-topic: pmrep: add xlsx support
Hi Marko,

----- Original Message -----
> Hi,
> 
> the patch below implements xlsx output support for pmrep, works both
> with live / archive inputs. Meaning that it now becomes trivial to pick
> up any PCP archive and general a spreadsheet out of it. Compared to the
> previously supported CSV format this is much more user friendly, after
> opening the file in Excel/LibreOffice one instantly sees the trends
> there and can start creating charts (with CSV files some hardcore
> spreadsheet skills are needed to get to this point).

Good stuff, well played!

> I checked the available Python XLSX modules at
> 
> http://www.python-excel.org/
> 
> and decided to use
> 
> https://pypi.python.org/pypi/XlsxWriter
> 
> It's way too heavy to be embedded (but it's trivial to install e.g.
> with pip anyway). It is actively maintained and has lots of features
> available if we want to create something fancier in the future.
> 
> We're a bit late in the release cycle but given how well contained this

Hmmm, more than a bit.  :)

> patch is I think it'd be nice to include it, this is a really nice

This is feature work, not soemthing for inclusion the day before a release.

This snippet also presents a problem from a packaging POV ...

> +try:
> +    import xlsxwriter
> +except:
> +    pass

We need to capture this dependency in the rpm packaging, for example, to
ensure that module is installed along with the script using it.  We should
also not require that dependency be installed for folk just wanting to use
the other pmrep output styles... (the "soft" dependency above is something
we have moved away from everywhere else in pcp, for many reasons).

This is part of the bigger high-level API problem I think i.e. that pmrep
shouldn't become a monolith, importing a new module for every new feature
- needs to be split into separate scripts.  So I think we need to fix that
aspect before adding more output formats ... this is all pcp-3.11.0 stuff
though, no rush there.

> feature. (Unrelated to anything else, this is a good example what an
> additional output alternative needs on pmrep level so gives a bit
> insight if we want to modularize things later.) Since the end result
> is completely dependent on xlsxwrite I'm not sure is PCP QA applicable
> here.

Heh, regression tests are always applicable - there has to be some way
of having confidence that this doesn't simply explode (eg. on py 3, or
next time someone changes pmrep in an accidentally overlapping area).

A basic test could simply export to xlsx and then check that the result
(file) is a valid document - that would give cursory py2/3 confidence.
A much better test would actually add some qa/src py code to verify the
contents of the generated doc.  Another alternative approach may be to
use sheet2pcp(1) to verify the result?  But, there must be some tests.


Also, those subsequent patches are dependent on this one - if they are
urgent fixes please rebase on master & resend?  (and tests, please - or
if qa is giving you hassles, send me the commands I can run to exercise
the fixes, and I'll create some regression tests for them - thanks!).

cheers.

--
Nathan

<Prev in Thread] Current Thread [Next in Thread>