I fixed the attribute issue above in 3cd2a8c. Can you take a look and see if this is what you were thinking?

Can you also take a look at the TODOs I put in lines 250 and 335 (same issue) and 462 of query.py and give me your opinion on those.

When that is done, I can finally merge this PR :)

Note:

• re the TODO in line 38: I will leave as is now and remove the #TODO
• re the TODO in line 525: I made a new issue (Make MapMethod an attribute of GridSettings in deeprank2.grid module #516) for this and will remove the #TODO

Also, should we remove codacy with this PR?

no, let's remove it in a separate PR? This one is already so big and unwieldly, that I don't want to add anything that doesn't need to be here.

Also, should we remove codacy with this PR?

no, let's remove it in a separate PR? This one is already so big and unwieldly, that I don't want to add anything that doesn't need to be here.

Done in #517

I fixed the attribute issue above in 3cd2a8c. Can you take a look and see if this is what you were thinking?

Can you also take a look at the TODOs I put in lines 250 and 335 (same issue) and 462 of query.py and give me your opinion on those.

When that is done, I can finally merge this PR :)

Note:

• re the TODO in line 38: I will leave as is now and remove the #TODO
• re the TODO in line 525: I made a new issue (Make MapMethod an attribute of GridSettings in deeprank2.grid module #516) for this and will remove the #TODO

line 250: I think it's actually fine to throw an error since only 1 chain identifier is needed to identify the chain where the variant is. If more are given then the user hasn't understood the purpose of chain_ids.
line 335: same
line 462: I don't see any TODO.

One final thing: looking at those lines made me think about something I commented earlier about, but was lost someway. I think it should be clearer that chain_ids refers to the chains involved in the definition of whatever query (e.g., the chain identifier of the variant), but does not limit the usage of other eventual chains present in the pdb files. It could be indeed that the radius or the cutoff distance defined is enough to include other chains, not considered in the chain_ids variable because its purpose is another one. So, for example in SingleResidueVariantQuery, the doc string says the chain identifier(s) in the pdb file (generally a single capital letter) but I think it should be something like the chain identifier of the variant residue in the pdb file (generally a single capital letter). For ProteinProteinInterfaceQuery, it could be something like the two chain identifiers of the interface in the pdb file (generally single capital letters).

Also, if there are more things to discuss let's just have a meeting because I have the impression that finding stuff in this PR is becoming very complex and inefficient ;)

I fixed the attribute issue above in 3cd2a8c. Can you take a look and see if this is what you were thinking?

Have you looked at this also?

Can you also take a look at the TODOs I put in lines 250 and 335 (same issue) and 462 of query.py and give me your opinion on those.

line 462: I don't see any TODO.

This is about error catching in _process_one_query:
Right now, if there's an error, it's caught and that query is skipped. My guess is that this was put in place so that it didnt crash due to a handfull of faulty pdb files.
My question was whether we want to make this optional, and by default raise an error. I think generally, users should have clean data to work with, and if not pro-actively set this to ignore errors. What do you think?

One final thing: looking at those lines made me think about something I commented earlier about, but was lost someway. I think it should be clearer that chain_ids refers to the chains involved in the definition of whatever query (e.g., the chain identifier of the variant), but does not limit the usage of other eventual chains present in the pdb files. It could be indeed that the radius or the cutoff distance defined is enough to include other chains, not considered in the chain_ids variable because its purpose is another one. So, for example in SingleResidueVariantQuery, the doc string says the chain identifier(s) in the pdb file (generally a single capital letter) but I think it should be something like the chain identifier of the variant residue in the pdb file (generally a single capital letter). For ProteinProteinInterfaceQuery, it could be something like the two chain identifiers of the interface in the pdb file (generally single capital letters).

Clarified in 23357f0