Add deprecation warning for ITPParser

[lilyminium]

Fixes #4698

Changes made in this Pull Request:

• Adds deprecation warning for element guessing in ITPParser

Please feel free to make any changes desired and push into this branch!

PR Checklist

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

Developers certificate of origin

• I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

---

📚 Documentation preview 📚: https://mdanalysis--4744.org.readthedocs.build/en/4744/

[pep8speaks]

Hello @lilyminium! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-10-24 18:11:49 UTC

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.62%. Comparing base (9b69745) to head (64804b6).
Report is 57 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4744      +/-   ##
===========================================
- Coverage    93.65%   93.62%   -0.03%     
===========================================
  Files          175      187      +12     
  Lines        21564    22631    +1067     
  Branches      3023     3023              
===========================================
+ Hits         20195    21189     +994     
- Misses         925      998      +73     
  Partials       444      444              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[IAlibay]

Couple of things, mostly clarifying the DeprecationWarning that is mostly user facing.

[IAlibay]

It would be best to not do this - i.e. we know this is prone to failures as soon as someone adds another warning somewhere else. However it's best to just fix it later.

[tylerjereddy]

You could also just use no_deprecated_call context manager I wrote for MDA 7 years ago in gh-1522.

[lilyminium]

Thanks Tyler -- trying the below code raised an error for me that I won't have time to look at for a few hours. Any quick suggestions? Otherwise since @IAlibay has called the feature freeze for his Sunday, I can raise an issue and fix this afterwards as he suggested.

def test_elements_nodeprecation_warning():
    "Test deprecation warning is not present if elements isn't guessed"
    with no_deprecated_call():
        mda.Universe(ITP_atomtypes)

raises

../util.py:244: in __exit__
    if any(issubclass(c, deprecation_categories) for c in self._captured_categories):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

.0 = <list_iterator object at 0x12cc235b0>

>   if any(issubclass(c, deprecation_categories) for c in self._captured_categories):
E   TypeError: issubclass() arg 1 must be a class

../util.py:244: TypeError

Edit: ah, the captured categories is a list of [None, None]. I had thought warnings.warn defaulted to UserWarning but maybe I'm confused here.

[tylerjereddy]

Fair enough, analysis below, maybe I'll open a ticket about this since I think I agree with you that the explicit requirement to add UserWarning shouldn't be needed (though it does fix the problem). Perhaps this is some subtlety about Python 2 at the time vs. 3 now:

this works just fine:

def test_elements_nodeprecation_warning():
    "Test deprecation warning is not present if elements isn't guessed"
    def foo():
        warnings.warn("boo", UserWarning)
    with no_deprecated_call():
        foo()

this fails like your report:

def test_elements_nodeprecation_warning():
    "Test deprecation warning is not present if elements isn't guessed"
    def foo():
        warnings.warn("boo")
    with no_deprecated_call():
        foo()

this patch gets the new test form working as you might expect from above analysis:

--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -145,7 +145,7 @@ def _resolve_coordinates(filename, *coordinates, format=None,
             get_reader_for(filename, format=format)
         except (ValueError, TypeError):
             warnings.warn('No coordinate reader found for {}. Skipping '
-                            'this file.'.format(filename))
+                            'this file.'.format(filename), UserWarning)
         else:
             coordinates = (filename,) + coordinates
     return coordinates
diff --git a/package/MDAnalysis/topology/ITPParser.py b/package/MDAnalysis/topology/ITPParser.py
index d2f7b6da8..ff47f0728 100644
--- a/package/MDAnalysis/topology/ITPParser.py
+++ b/package/MDAnalysis/topology/ITPParser.py
@@ -607,7 +607,7 @@ class ITPParser(TopologyReaderBase):
             warnings.warn("Element information is missing, elements attribute "
                           "will not be populated. If needed these can be "
                           "guessed using universe.guess_TopologyAttrs("
-                          "to_guess=['elements']).")
+                          "to_guess=['elements']).", UserWarning)

[lilyminium]

Thanks Irfan, committed the changes you suggested!