Skip to content

Conversation

@mgiulini
Copy link
Contributor

@mgiulini mgiulini commented Jun 20, 2022

You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and you comply with the following criteria:

  • You have stick to Python. Talk with us before adding other programming languages to HADDOCK3
  • Your PR is about CNS
  • Your code is well documented: proper docstrings and explanatory comments for those tricky parts
  • You structured the code into small functions as much as possible. You can use classes if there's a (state) purpose
  • code follows our coding style
  • You wrote tests for the new code
  • tox tests pass. Run tox command inside the repository folder
  • -test.cfg examples execute without errors. Inside examples/ run python run_tests.py -b
  • PR does not add any install dependencies unless permission granted by the HADDOCK team
  • PR does not break licensing
  • Your PR is about writing documentation for already existing code 🔥
  • Your PR is about writing tests for already existing code :godmode:

Closes #472 by adding a call to an AlignError. It does not stop caprieval, but rather the capri_ss.tsv file is still produced for all those models that do not show an evident (empty chain) chain-matching error.

Copy link
Member

@joaomcteixeira joaomcteixeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Minor comments below! (Sorry i edited my original message)

min_idx = min(chain_dic[chain])
max_idx = max(chain_dic[chain])
chain_ranges[chain] = (min_idx, max_idx)
if chain_dic[chain] == []:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the pure pythonic way of doing it, I suggest:

if not chain_dic[chain]:

or

if len(chain_dic[chain]) == 0:

max_idx = max(chain_dic[chain])
chain_ranges[chain] = (min_idx, max_idx)
if chain_dic[chain] == []:
raise ALIGNError(f"Chain matching error on {pdb_f}, chain {chain}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can, please add a minor test covering the new lines:

def test_new_test_function():
    with pytest.raises(ALIGNError):
        load_coords(...)

Copy link
Member

@joaomcteixeira joaomcteixeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

@mgiulini mgiulini merged commit 89653c1 into main Jun 20, 2022
@mgiulini mgiulini deleted the iss472 branch November 16, 2022 11:30
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.

clarify chain matching error in caprieval

3 participants