Align pdb #165
Align pdb #165
Conversation
| input_arg, | ||
| input_uniprot, | ||
| input_fasta, | ||
| input_pdb, |
There was a problem hiding this comment.
main takes quite a lot of arguments, it might be time to start considering using a configuration file instead of command line arguments.
There was a problem hiding this comment.
I agree, but this was probably the last (or second last) big modification to the code, so I would not change the whole input structure for the moment.
| input_files = {} | ||
| # retrieve uniprot information | ||
| if inp.is_fasta(): | ||
| input_files["fasta"] = Path(inp.arg) | ||
| uniprot_id = run_blast(input_files["fasta"], db) | ||
| if inp.is_uniprot(): | ||
| uniprot_id = inp.arg.upper() | ||
| if inp.is_pdb(): | ||
| input_files["pdb"] = Path(inp.arg) | ||
| uniprot_id = None | ||
| # fasta input | ||
| if is_fasta(input_fasta): | ||
| input_files["fasta"] = Path(input_fasta) | ||
| if not is_uniprot(input_uniprot): | ||
| uniprot_id = run_blast(input_files["fasta"], db) | ||
| else: | ||
| log.warning("Uniprot ID provided. The fasta file will be ignored.") | ||
| # pdb input | ||
| if is_pdb(input_pdb): | ||
| input_files["pdb"] = Path(input_pdb) | ||
| if not interface_file: | ||
| fasta_f = to_fasta(input_files["pdb"], temp=False) | ||
| uniprot_id = run_blast(fasta_f.name, db) | ||
| if not is_uniprot(input_uniprot): | ||
| uniprot_id = run_blast(fasta_f.name, db) | ||
| # remove fasta_f | ||
| input_files["pdb"].with_suffix(".fasta").unlink() | ||
| else: | ||
| input_files["interface_file"] = Path(interface_file) | ||
| uniprot_id = None | ||
| # 25/7/2023: if the pdb is provided, the blast uniprot_id can be | ||
| # overwritten by specifying the uniprot_id input argument | ||
| if is_uniprot(input_uniprot): | ||
| uniprot_id = input_uniprot.upper() |
There was a problem hiding this comment.
Since you are using only if and not if/elif/else, uniprot_id can be None by the end of the switch. Here you should either re-work this logic and add hierarchy to the parameters (with if/elif/else) or add an extra check that uniprot_id is not None
There was a problem hiding this comment.
but it's fine to have uniprot_id = None at the end of the switch!
| input_files["pdb"], uniprot_id | ||
| ) | ||
| else: | ||
| pdb_f = input_files["pdb"] | ||
| else: | ||
| if pdb_data: | ||
| pdb_data_path = input_files["pdb_data"] |
There was a problem hiding this comment.
Keep in mind that here if pdb_data_path is None the get_best_pdb function will raise an IndexError
| with open(pdb_renum, "w") as wfile: | ||
| with open(pdb_torenum) as rfile: | ||
| for ln in rfile: |
There was a problem hiding this comment.
There's too much identantion here, see if you can reduce it. You should be able to do things such as: with open(pdb_renum, "w") as wfile, with open(pdb_torenum) as rfile: and instead of doing the if check and going into the block you can do it the other way around and continue if it does not pass the check
Closes #124