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

Remove eval when asking user for confirmation to delete existing directories #20

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benhg
Copy link

@benhg benhg commented Oct 27, 2022

Hello,

I'm not sure the use of eval here is accomplishing exactly what you want it to. Since eval is directly evaluating whatever python code a user gives you, and you are then assigning that directly to the value of a variable, that means the user has to give a valid Python expression that evaluates to "something storable".

For example, if a user were to just give a Y or N rather than the literal "Y" with the quotes, something like this would happen:

(phyluce-conda) glick@mayo:/home/labs/binford/uce-tutorial$ illumiprocessor \
>     --input raw-fastq/ \
>     --output clean-fastq \
>     --config illumiprocessor.conf \
>     --cores 4
[WARNING] Output directory exists, REMOVE [Y/n]? y
Traceback (most recent call last):
  File "/local/cluster/conda/envs/phyluce-conda/bin/illumiprocessor", line 17, in <module>
    sys.exit(main())
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/site-packages/illumiprocessor/cli/main.py", line 113, in main
    args = get_args()
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/site-packages/illumiprocessor/cli/main.py", line 107, in get_args
    return parser.parse_args()
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/argparse.py", line 1734, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/argparse.py", line 1766, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/argparse.py", line 1972, in _parse_known_args
    start_index = consume_optional(start_index)
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/argparse.py", line 1912, in consume_optional
    take_action(action, args, option_string)
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/argparse.py", line 1840, in take_action
    action(self, namespace, argument_values, option_string)
  File "/local/cluster/conda/envs/phyluce-conda/lib/python3.6/site-packages/illumiprocessor/core.py", line 40, in __call__
    answer = eval(input("[WARNING] Output directory exists, REMOVE [Y/n]? "))
  File "<string>", line 1, in <module>
NameError: name 'y' is not defined

because now Python is trying to evaluate what the literal python code y means.

input() should already take care of converting user input to a Python string, so I think it should be safe to simply use the value returned by input()

It's very possible that I'm misunderstanding your intentions, so please feel free to close this PR if this behavior is intended.

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

Successfully merging this pull request may close these issues.

1 participant