[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [MoM] phyx upstream update & changes (Was: Re: [MoM] lefse migration to python 3)



Hi Shayan,

On Fri, Sep 13, 2019 at 03:07:38AM +0100, Shayan Doust wrote:
> The issue was due to precision and how the test factored this in when it
> came to check / comparison. Upstream has fixed this so all 41 tests pass
> on my system.

I confirm all tests are passing here as well.  Very good that this ended
up in a fruitful cooperation with upstream.  Admitedly I would have loved
even more if upstream would have merged our two patches (Python3 and the
additional flags in Makefile.in (I wonder why upstream does not run into
the same problem with missing -lnlopt_cxx -lnlopt options).

At least the Python3 patch should be mandatory for the next release.

> I left your current autopkgtest implementation as it is
> late and I didn't want to remove your work so integrate this in.

Well, for sure everybody is free to replace my work by something better.
I admit it was not so much work since it was basically using our
template plus grep | sed on the docs (well, some manual things remained
but it was basically a no brainer).

However, the pro of my autopkgtest is that it is based on the user
example data that are provided in the docs anyway and the test script
is provided to serve as an additional example script.

The con is that I'm not sure at all whether the results of the programs
are correct since I have the feeling that these do not issue proper exit
codes neither is the resulting output compared to anything.  So the
runtime test is probably more precise - but to stick to our strategy to
enable users running it we would need to add the test data to the
packaging - may be in some different package to not bloat the actual
package to much.  So we can leave this for a later release since its
probably over-engineering at the moment while other important work would
be left behind.  I simply droped a remark about this in the test script.

Thanks again for your thorough and really helpful work.  This brings
up the idea whether you want to become a DM (Debian Maintainer).  You
paced up really quickly through MoM into an active member of the team.

Kind regards

      Andreas.


> Kind regards,
> Shayan Doust
> 
> On 11/09/2019 21:35, Andreas Tille wrote:
> > Hi Shayan,
> > 
> > On Wed, Sep 11, 2019 at 05:34:19PM +0100, Shayan Doust wrote:
> >>> plot_cladogram comparative fail
> >>
> >> I cannot reproduce this and I just did a clean build and installed a
> >> fresh *.deb. I did however do some refinements and any fails will result
> >> in script execution which will display the output so please pull.
> > 
> > Ahhh,
> > 
> > plot_cladogram comparative fail. Instead, I get:
> > Traceback (most recent call last):
> >   File "/usr/bin/plot_cladogram", line 5, in <module>
> >     from pylab import *
> > ModuleNotFoundError: No module named 'pylab'
> > 
> > 
> > This uncovered a new missing Depends!  This again proves the sense
> > of even very simple tests.
> > 
> >>> I've observed your commits to phyx but did not acted upon it since you
> >>> did not confirmed that you are ready here.
> >>
> >> More about this: this is ready. Both the run_tests.py and gui interface
> >> work. A note about the run_tests.py. Before the conversion, 9 tests
> >> failed (I think 31 passed) and after the conversion, there is the same
> >> failure rate.
> > 
> > Confirmed according to
> > 
> >    https://buildd.debian.org/status/fetch.php?pkg=phyx&arch=amd64&ver=0.999%2Bds-1&stamp=1539857991&raw=0
> > 
> >> Not sure if this is some upstream fault because I have not
> >> been able to get a fresh upstream copy and directly test the
> >> run_tests.py, but I assume it will be the same as what I get now.
> > 
> > That's correct - may be we should contact upstream about this anyway.  I
> > simply created an autopkgtest from Phyxed_Manual.tex (via sed + minor
> > editing).  If the tools send correct exit codes this should work.
> > 
> > Kind regards
> > 
> >        Andreas. 
> > 
> 




-- 
http://fam-tille.de


Reply to: