Conversation
Signed-off-by: VishnuR23 <vishnurajeev2345@gmail.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: NVIDIA-BioNeMo/bionemo-recipes/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
| count = fasta_to_jsonl(args.input, args.output, uppercase=args.upper) | ||
| try: | ||
| count = fasta_to_jsonl(args.input, args.output, uppercase=args.upper) | ||
| except ValueError as error: | ||
| print(f"Error: {error}", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
I'm pretty sure this doesn't add anything, right? the stack trace will be printed either way and the program will exit on that raise. Otherwise looks good.
jstjohn
left a comment
There was a problem hiding this comment.
Approved with one cleanup request around exception handling (I think it would be better to not do it in this case), since as far as I can tell all you are doing is prepending the stack trace message with the word "Error: ".
Description
Passing the same file as both input and output to
bionemo_fasta_to_jsonlopens the output in write mode before reading the source, silently erasing the FASTA and reporting zero records. Different path strings can trigger the same loss through symlinks or hard links.Check file identity before opening the output and raise a clear
ValueErrorwhen input and output refer to the same file. The CLI reports the error and exits with status 1. Conversion to a distinct output retains the current overwrite and uppercase behavior. Regenerate the enforced Eden/phage copies.Type of changes
Validation