Add type annotations to all functions and methods #1179
Conversation
| from cclib.parser.utils import find_package | ||
|
|
||
| _found_psi4 = find_package("psi4") | ||
| if _found_psi4: | ||
| from psi4.core import Molecule | ||
|
|
||
|
|
||
| def _check_psi4(found_psi4): | ||
| def _check_psi4(found_psi4: bool) -> None: |
There was a problem hiding this comment.
Do you think it's prudent to annotate internal functions? Something I normally don't do.
There was a problem hiding this comment.
This comment should have been a response here: #1179 (comment):
I always do, since they may be reused many times, the body won’t get
checked if the signature isn’t annotated, and I’ve been bitten by this
before.
|
I always do, since they may be reused many times, the body won’t get
checked and f the signature isn’t annotated, and I’ve been bitten by this
before.
…On Sat, Mar 4, 2023 at 5:03 PM Karol M. Langner ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cclib/bridge/cclib2psi4.py
<#1179 (comment)>:
> from cclib.parser.utils import find_package
_found_psi4 = find_package("psi4")
if _found_psi4:
from psi4.core import Molecule
-def _check_psi4(found_psi4):
+def _check_psi4(found_psi4: bool) -> None:
Do you think it's prudent to annotate internal functions? Something I
normally don't do.
—
Reply to this email directly, view it on GitHub
<#1179 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFRUEYNZA4G4VJGLN3NYXLW2O34LANCNFSM6AAAAAAVPYTITE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
| additional_dependencies: | ||
| - numpy | ||
| - packaging | ||
| - pandas-stubs |
There was a problem hiding this comment.
Not obvious to me why pandas is a dep.
There was a problem hiding this comment.
Line 496 in 35d72b0
There was a problem hiding this comment.
I should say exactly what this file is. It's technically not necessary for mypy to run.1 .pre-commit-config.yaml is a configuration file for https://pre-commit.com/, which is two things:
- A tool that runs linters and fixers from the command line. You do not need to have it as a pre-commit hook. Just doing https://pre-commit.com/#4-optional-run-against-all-the-files is enough.
- A free CI service that runs alongside GH workflows, ReadTheDocs, etc.
The benefit to having 1 is that everyone has a unified way of running all these tools that's independent from CI, specifically opaque GitHub Actions. You can have 1 without 2. 2 would mean that you get the benefits of the hook (clean repository) without forcing contributors to install the hook.
The reason pandas-stubs appears in additional_dependencies is because of how the tool works. For Python-based tools, it creates a fresh venv to run that tool in, and by default it only installs that tool. This means that mypy will not have access to any 3rd-party type information unless the dependencies or packages with only type stubs (for dependencies that don't ship them together) are also installed.
Footnotes
-
For mypy configuration, you only need
pyproject.tomlormypy.ini. I chose the former since everything is moving toward it anyway. ↩
|
I think what I'll do is convert this from draft to PR once CI is fixed and leave everything as-is, so at least once more round of adding annotations, fixing annotations (wrong ones, fixing |
Not code style exactly, but I see it as part of #1122.