Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command_line/reduce.py ignoring return value from reduce #1

Open
wojdyr opened this issue Feb 19, 2016 · 2 comments
Open

command_line/reduce.py ignoring return value from reduce #1

wojdyr opened this issue Feb 19, 2016 · 2 comments

Comments

@wojdyr
Copy link

wojdyr commented Feb 19, 2016

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
Copy link
Collaborator

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 [email protected]
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
#1.

@wojdyr
Copy link
Author

wojdyr commented Feb 22, 2016

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.

russell-taylor added a commit that referenced this issue Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants