issue #2469

[siddharthjain1611]

Fixes #2469

Changes made in this Pull Request:
keeps match_atoms=True
get_matching_atoms was looking for 'mismatching' atoms by checking if anyone pair has a difference higher than tol_mass. This change keeps match_atoms=True.

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[siddharthjain1611]

Line where the code was changed

[xiki-tempula]

It might be better if this line stays the same and a warning is issued when no mass can be found (potentially via another except).
The format will be similar to warnings.warn(msg, category=SelectionWarning).
Please read the doc to check the types of warnings available and think of a meaningful message.

[xiki-tempula]

I felt that instead of just silently skipping the match_atom test.
Adding another except AttributeError and then issue a warning saying that mass match has been skipped due to no mass would be better.
Besides, you probably need to fix the test.

[orbeckst]

@xiki-tempula thank you for looking over the PR, your comments are very helpful.

For reviewing gsoc-related PRs it would be super-helpful if you did a code review (instead of just posting a comment) and then select "request changes" to be extra clear what is expected of students. You can always ping another core dev for a second opinion and one of the @MDAnalysis/gsoc-mentors / core devs will have to sign off on the PR anyway.

[siddharthjain1611]

Thank you so much @xiki-tempula sir and @orbeckst sir for your suggestions, Please allow me to work on this and improve this further.

[orbeckst]

What was the reason for closing the PR?

[siddharthjain1611]

sorry sir, but I thought that I had done something terribly wrong, therefore in order to correct my mistake with a new pr, I closed this.
Anyways, sir, it would be a great help if you could help me out with the resources which can help me out with testing the changes locally.

[orbeckst]

What is the number of the new PR?

[siddharthjain1611]

I've not created one though but I suppose creating an Attribute Error except would solve the problem. Please correct if this is incorrect. I would love your suggestions as I'm a beginner to Molecular Dynamics in Python. I'll create a PR soon if you agree to the above suggestion.

[xiki-tempula]

@siddharthjain1611 The idea seems sound to me.
I think the expected behaviour is that when there is no mass. The program executes but issue a warning saying that the mass check has been skipped instead of an error.
The rationale is that for some systems, the mass is not present. However, the program should still run as there is no reason why it shouldn't. However, since the mass check has been skipped, the user need to be warned of the possibility that the atoms are not matched as they thought.

[RMeli]

@siddharthjain1611

The User Guide contains a lot of information about setting up a developer environment, running tests locally and writing new tests.

In short, you will need to install pytest to run the tests in testsuite/MDAnalysisTests.

[siddharthjain1611]

except AttributeError:
errmsg=("Failed to check the mass matches")
raise_from(SelectionError(errmsg), None)
I think that above code should fix the issue.

[siddharthjain1611]

I've created a PR #2614. Please suggest if any changes required.