Code review comment for lp:~maddevelopers/mg5amcnlo/FixMA5

Revision history for this message
Valentin Hirschi (valentin-hirschi) wrote :

On Thu, Nov 23, 2017 at 12:53 PM, Olivier Mattelaer <
<email address hidden>> wrote:

> Review: Approve
>
> Hi Valentin,
>
> Perfect, nicely done. just one minor comment to fix before merging.
>
> 1)
> @@ -2755,6 +2760,7 @@
> shutil.copy(target,
> pjoin(self.me_dir,'Events',self.run_name,carboncopy_name))
> else:
> logger.error('MadAnalysis5 failed to create PDF output')
> + stop
>
> stop is not a valid python syntax. Is that a debug statement that you
> forget to remove?
>

Yes, thank you for having spotted this. I pushed the removal of this line.

>
>
>
>
>
> Diff comments:
>
> >
> > === modified file 'madgraph/interface/common_run_interface.py'
> > --- madgraph/interface/common_run_interface.py 2017-11-17
> 12:35:40 +0000
> > +++ madgraph/interface/common_run_interface.py 2017-11-21
> 13:27:59 +0000
> > @@ -2755,6 +2760,7 @@
> > shutil.copy(target,
> pjoin(self.me_dir,'Events',self.run_name,carboncopy_name))
> > else:
> > logger.error('MadAnalysis5 failed to create PDF output')
> > + stop
>
> Well stop is not a valid python syntax -> should be raise Exception
> "MadAnalysis5 failed to create PDF output" if you really want to make the
> code to crash (but do we want that?)
>
> > if MA5_runtag!='default':
> > logger.info("MadAnalysis5 successfully completed the "+
> > "%s. Reported results are placed in:"%("analysis
> '%s'"%MA5_runtag
>
>
> --
> https://code.launchpad.net/~maddevelopers/mg5amcnlo/FixMA5/+merge/333963
> Your team MadDevelopers is subscribed to branch
> lp:~maddevelopers/mg5amcnlo/FixMA5.
>

--
Valentin
--
Valentin

« Back to merge proposal