command_line/reduce.py ignoring return value from reduce

[wojdyr]

the reduce.py script calls reduce but doesn't pass it's return value, see https://github.com/rlabduke/reduce/blob/master/command_line/reduce.py#L20 So reduce has sometimes return status 1, but phenix.reduce always returns 0. This change would be trivial to change, like this:

  return subprocess.call(cmd + args)
  ...
  sys.exit(run(sys.argv[1:]))

unless ignoring the status code is actually intentional - I can't be sure about it.

[chrissciwilliams]

Hi Marcin,

Thanks for catching this (and double thanks for providing a suggestion). We'll look into it, and in the process make sure we're playing nice with subprocess calls inside the phenix/cctbx environment.

As a note to fellow developers - the recently added error catching for mp_geo makes use of the mp_geo return code. If we want to add more error catching checkpoints, it's very much in our interest to make sure that return codes behave in an expected way.

-Christopher Williams ---Richardson Lab, Duke University

On Fri, Feb 19, 2016 at 2:14 PM, Marcin Wojdyr notifications@github.com wrote:

the reduce.py script calls reduce but doesn't pass it's return value, see https://github.com/rlabduke/reduce/blob/master/command_line/reduce.py#L20 So reduce has sometimes return status 1, but phenix.reduce always returns 0. This change would be trivial to change, like this:

return subprocess.call(cmd + args) ... sys.exit(run(sys.argv[1:]))

unless ignoring the status code is actually intentional - I can't be sure about it.

— Reply to this email directly or view it on GitHub https://github.com/rlabduke/reduce/issues/1.

[wojdyr]

another option would be to remove reduce.py and have only auto-generated dispatcher, like it is done for probe. I think the only difference would be that the dispatcher would need to set REDUCE_HET_DICT.