Use match to check hole2 warnings

[lilyminium]

Fixes #

Tests are failing in #3160 because tests for hole2 check how many warnings are generated. This has been a pain in the past as we add and remove warnings.

Changes made in this Pull Request:

• Removes the length of the warning check
• Instead, checks for the desired warning by checking the message

PR Checklist

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

[IAlibay]

536 extra errors is problematic, we need to get to the bottom of that first?

I can't imagine it being healthy for users to have a wall of warnings show up?

[codecov]

Codecov Report

Merging #3162 (3b21981) into master (7ed893d) will increase coverage by 0.84%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3162      +/-   ##
==========================================
+ Coverage   90.97%   91.81%   +0.84%     
==========================================
  Files         162      167       +5     
  Lines       22194    22699     +505     
  Branches     3198     3198              
==========================================
+ Hits        20190    20841     +651     
- Misses       1381     1774     +393     
+ Partials      623       84     -539     

Impacted Files	Coverage Δ
package/MDAnalysis/topology/tpr/obj.py	96.82% <0.00%> (-3.18%)	⬇️
...onality_reduction/DimensionalityReductionMethod.py	97.14% <0.00%> (-2.86%)	⬇️
package/MDAnalysis/analysis/legacy/x3dna.py	0.00% <0.00%> (ø)
package/MDAnalysis/tests/datafiles.py	31.25% <0.00%> (ø)
package/MDAnalysis/tests/__init__.py	100.00% <0.00%> (ø)
package/MDAnalysis/analysis/legacy/__init__.py	0.00% <0.00%> (ø)
package/MDAnalysis/due.py	56.09% <0.00%> (ø)
package/MDAnalysis/core/selection.py	99.52% <0.00%> (+<0.01%)	⬆️
...sis/analysis/encore/clustering/ClusteringMethod.py	96.96% <0.00%> (+0.04%)	⬆️
package/MDAnalysis/analysis/base.py	95.60% <0.00%> (+0.04%)	⬆️
... and 85 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ed893d...3b21981. Read the comment docs.

[lilyminium]

@IAlibay I think it's because we get the bonds for each atom. There's 264 atoms, and the DeprecationWarning in #3160 is raised each time we get the bonds and it's not an empty group. That'll be a pain for writing PDB files out, I guess -- although I think the default treatment of multiple warnings is to only print out the first instance.

[IAlibay]

@IAlibay I think it's because we get the bonds for each atom. There's 264 atoms, and the DeprecationWarning in #3160 is raised each time we get the bonds and it's not an empty group. That'll be a pain for writing PDB files out, I guess -- although I think the default treatment of multiple warnings is to only print out the first instance.

Yikes :( does this incur an additional performance cost?

[lilyminium]

Yikes :( does this incur an additional performance cost?

Incrementally more than the cost associated with getting the bonds atom-by-atom. I could probably update the writer to just call bonds once.

[IAlibay]

If it's a tiny cost increase I have no issues, I think it's worth pinging @jbarnoud and @orbeckst who might have stronger views on warnings & performance.

[orbeckst]

I am happy with this PR; checking exactly what you're looking for than just checking numbers of warnings is better in any case.

The root cause issue of having all these additional warnings requires another issue. Let's keep things simple.

[orbeckst]

@IAlibay please merge when you think it's ready.

[IAlibay]

I am happy with this PR; checking exactly what you're looking for than just checking numbers of warnings is better in any case.

The root cause issue of having all these additional warnings requires another issue. Let's keep things simple.

Ok let's go with this then.

We should think of a way to test how many warnings get emitted during our tests (i.e. something like codecov for warnings), that way we can tell just how much of an impact adding warnings have.