Skip to content

Conversation

@jchorl
Copy link

@jchorl jchorl commented Sep 22, 2024

I'd recommend reviewing commit-by-commit, I tried to keep them isolated and understandable. In order:

  1. Switch from passing concatenated strings to subprocess.check_output to passing lists to subprocess.run. The reason is that "my_cool_app " + input_file will break if input_file contains a space or a myriad of other special chars. This can also enable vulnerabilities if e.g. a file is named || true; rm -rf /
  2. Adopt pathlib.Path for cwd, it's easier to do path operations and seems to be a successor for os.path
  3. Similarly, type args with Path and int and bool. Might as well rely on argparse to properly parse types.
  4. Just removing an unnecessary comment
  5. I ran the ruff linter and got it passing on run.py. I noticed mixing 2 vs. 4 space indentation, some unused vars, etc. The linter put the code in better shape.

I only got to run.py as some of the other files are bigger and more work. But hopefully this helps. Tested on python3.12.

@jchorl
Copy link
Author

jchorl commented Sep 22, 2024

Note there are a couple breaking changes here:

  • -k/--keep uses BooleanOptionalAction, so the flags become -k/--keep and --no-keep (as opposed to -k true or -k false)
  • Same for -r/--repeats

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