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

Issue with calling subprocess command #66

Open
fwzhao opened this issue Jun 10, 2020 · 2 comments
Open

Issue with calling subprocess command #66

fwzhao opened this issue Jun 10, 2020 · 2 comments

Comments

@fwzhao
Copy link
Collaborator

fwzhao commented Jun 10, 2020

toolkit/ngs_toolkit/general.py

Lines 1400 to 1408 in 3f828b2

# shuffle input in no background is provided
if background_fasta is None:
shuffled = input_fasta + ".shuffled"
cmd = """
fasta-dinucleotide-shuffle -c 1 -f {0} > {1}
""".format(
input_fasta, shuffled
)
subprocess.call(cmd.split(" "))

The new lines ("\n") and write std_out (">") are causing problems for subprocess.call
I originally encountered this issue when running meme_ame having broken down the command, wehre first there was a Unknown command argument: "\n": "\n", which i was able to trace to the -
"""
cmd
"""
Subsequently after fixing this, subprocess.call does not recognize ">"

The fix:

shuffled = input_fasta + ".shuffled"
file = open(shuffled, "w")
cmd = """fasta-dinucleotide-shuffle -c 1 -f {0}""".format(
    input_fasta
)
subprocess.call(cmd.split(" "), stdout=file)
file.close()

This actually generates the shuffled background. I'm not sure why ">" is not accepted, I did some googling but it was not successful

@afrendeiro
Copy link
Owner

Hi, thanks for investigating this.
Yeah that does make sense as that is at least missing a shell=True call.

I've recently come up with a improved function to call subprocess.call (https://github.com/ElementoLab/imcpipeline/blob/master/imcpipeline/utils.py#L158) and will likely port that over to ngs_toolkit.

@fwzhao
Copy link
Collaborator Author

fwzhao commented Jun 25, 2020

Yes, making it shell=True also could work, but the consensus on stackoverflow was that it's a bit riskier to use shell=True (in certain cases, accidental user input can mess things up), hence the stdout file approach.

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