Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type hints in the public API #1530

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

texasaggie97-zz
Copy link
Contributor

@texasaggie97-zz texasaggie97-zz commented Nov 10, 2020

  • This contribution adheres to CONTRIBUTING.md.
  • I've updated CHANGELOG.md if applicable.
  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

  • Add type hints to public API
    • Uses type_in_documentation metadata since it mostly has the information needed - we need the complete list of possible types in the API, not just the type that is passed into the DLL
    • Several special cases to deal with free form test
    • Not sure how to put this information into the metadata
  • Most type hints are calculated
  • A few in fancy functions are hardcoded
  • The preferred method for type hinting namedtuples is to create a class that inherits from typing.NamedTuple. This would require the couple of namedtuples we have to be moved to custom types. I did not do that work for this PR.

List issues fixed by this Pull Request below, if any.

What testing has been done?

  • Unit tests and flake8 still pass with type hinting added
  • Need to add mypy tests, maybe using examples?

@codecov
Copy link

codecov bot commented Nov 10, 2020

Codecov Report

Merging #1530 (28c9e39) into master (be1d5aa) will decrease coverage by 0.33%.
The diff coverage is 87.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1530      +/-   ##
==========================================
- Coverage   91.66%   91.33%   -0.34%     
==========================================
  Files          20       20              
  Lines        3457     3510      +53     
==========================================
+ Hits         3169     3206      +37     
- Misses        288      304      +16     
Flag Coverage Δ
codegenunittests 87.75% <64.70%> (-0.52%) ⬇️
nifakeunittests 96.35% <100.00%> (+<0.01%) ⬆️
nimodinstunittests 95.40% <100.00%> (+0.02%) ⬆️
nitclkunittests 95.65% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
build/helper/codegen_helper.py 90.22% <7.69%> (-2.30%) ⬇️
build/helper/metadata_add_all.py 81.85% <83.78%> (+0.50%) ⬆️
build/helper/__init__.py 100.00% <100.00%> (ø)
generated/nifake/nifake/session.py 97.71% <100.00%> (+<0.01%) ⬆️
generated/nimodinst/nimodinst/session.py 95.40% <100.00%> (+0.02%) ⬆️
generated/nitclk/nitclk/session.py 95.65% <100.00%> (+0.03%) ⬆️

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 be1d5aa...28c9e39. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant