Providing type hints for core module

[umak1106]

Fixes #

Changes made in this Pull Request:

PR Checklist

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

[pep8speaks]

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

• In the file package/MDAnalysis/core/_get_readers.py:

Line 38:80: E501 line too long (111 > 79 characters) Line 117:80: E501 line too long (113 > 79 characters) Line 213:80: E501 line too long (121 > 79 characters)

• In the file package/MDAnalysis/core/accessors.py:

Line 205:80: E501 line too long (83 > 79 characters)

Comment last updated at 2022-07-04 19:40:35 UTC

[umak1106]

When I run mypy on this file it runs without any errors . But I am unable to figure out what class name should be used instead of ConverterWrapper in t.Type[Converterwrapper]. Also if there are any other mistakes in this please let me know so .

[jbarnoud]

I'll have a look on Monday :)

[umak1106]

We use a bunch of library-level dictionaries that link a format name to a class (e.g. _READERS, _PARSERS). These cannot be used as type hints as they are not classes. The fact that mypy did not complain at any point makes me think that there are issues in the way we run the type analysis. How do you run mypy to test you changes to these files? Did you remove them from the exclusions?

The modules you are working on now are defining things that are called elsewhere. You need to run the analysis on places where these methods are called. The tests are likely a good place for that, actually.

On a different note. Could you please avoid to force push? When you force the a git push, you make it more difficult for us to review you code for 2 reasons:

* if we loaded your branch on our computer, your force push can create issues when we try to upgrade to your latest changes

* we do not see the evolution of your changes, so it is very difficult to know what are the last things you changed.

I removed mypy.ini and mypy.yaml from my local repo and ran this command mypy --follow-imports=skip _get_readers.py which is why it skipped those errors

If i set --follow-imports=normal I do get errors regarding these . _get_readers.py:25: error: Skipping analyzing "mmtf": module is installed, but missing library stubs or py.typed marker _get_readers.py:25: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports _get_readers.py:34: error: Variable "MDAnalysis._READERS" is not valid as a type _get_readers.py:34: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases _get_readers.py:113: error: Variable "MDAnalysis._SINGLEFRAME_WRITERS" is not valid as a type etc..

I used to squash merge my commits for a cleaner commit history but i'll avoid force pushing now

[jbarnoud]

I would suggest you run mypy on the tests, this is where the objects get used and therefore where you get to evaluate your annotations. Also, at least when running locally, I would avoid silencing the errors too much; that requires filtering the output when reading it, but it avoids missing important messages.