Skip to content

Conversation

@alchemistcai
Copy link
Contributor

Add most function and variable annotations for haddock.

Add src/haddock/core/typing.py to contain relative definations.

Adjust some functions to accept both Path and str when docstrings say they accept Path and strwhile the code only accept str, and other situations like this.

Alter some lists to generators when they are used only once.

Known issues:I can't infer types of haddock.libs.libontology.PDBFile.topology, ModuleIO.input,ModuleIO.output and relative functions.They seem to have many types.

I have to say that adding annotations for haddock is not an esay job,maybe even not a good idea.Feel free to join this discussion to show your opinion about type annotations #637 👀 .

@rvhonorato rvhonorato requested review from mgiulini and rvhonorato May 1, 2023 16:28
@rvhonorato rvhonorato added the enhancement Improving something in the codebase label May 1, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2023

Codecov Report

Attention: 124 lines in your changes are missing coverage. Please review.

Comparison is base (4b38f64) 74.97% compared to head (7f92406) 74.94%.

❗ Current head 7f92406 differs from pull request most recent head cf88e3d. Consider uploading reports for the commit cf88e3d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           typing     #645      +/-   ##
==========================================
- Coverage   74.97%   74.94%   -0.04%     
==========================================
  Files         113      114       +1     
  Lines        8048     8177     +129     
==========================================
+ Hits         6034     6128      +94     
- Misses       2014     2049      +35     
Files Coverage Δ
src/haddock/__init__.py 96.29% <100.00%> (ø)
src/haddock/clis/cli_analyse.py 74.69% <100.00%> (+0.30%) ⬆️
src/haddock/clis/cli_cfg.py 81.81% <100.00%> (+0.42%) ⬆️
src/haddock/clis/cli_cp.py 80.85% <100.00%> (+0.41%) ⬆️
src/haddock/core/cns_paths.py 100.00% <100.00%> (ø)
src/haddock/core/defaults.py 88.88% <ø> (ø)
src/haddock/core/supported_molecules.py 93.52% <100.00%> (+0.14%) ⬆️
src/haddock/gear/clean_steps.py 87.32% <100.00%> (ø)
src/haddock/gear/config.py 79.79% <100.00%> (+0.20%) ⬆️
src/haddock/gear/expandable_parameters.py 100.00% <100.00%> (ø)
... and 55 more

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

@rvhonorato
Copy link
Member

rvhonorato commented May 1, 2023

Thanks a lot @alchemistcai for putting the effort on this!

I not only think that adding the types is a good idea but I also think we should add mypy to the CI actions, but this is not the most popular opinion in the haddock team.

I'll go over your changes in the coming days, for now I've merged the main into this PR since there were some conflicts after #649

*I've also added a types env to tox to make it easier to check it

@rvhonorato rvhonorato added the community Feature requests / contribution / issues from users label May 10, 2023
@rvhonorato rvhonorato changed the base branch from main to typing October 16, 2023 13:27
@rvhonorato
Copy link
Member

rvhonorato commented Oct 16, 2023

Sorry for the delay in handling this one.

It has become a huge atomic PR, so I have rebased it to haddocking:typing and will continue in a separate PR to check for issues (so we can run the full set of actions)

Thanks again @alchemistcai!

@rvhonorato rvhonorato merged commit 1002963 into haddocking:typing Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Feature requests / contribution / issues from users enhancement Improving something in the codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants