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

fix(mypy): addresses lack of optional types #521

Merged
merged 10 commits into from
Jul 16, 2021

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Jul 12, 2021

Issue #, if available:

Description of changes:

Resolve mypy errors, down from 124 to 30 :-) .

Changes:

  • Resolve errors related to the "no_implicit_optional" flag in mypy.ini
  • Fix typing for resolve_truthy_env_var_choice

Checklist

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Resolve errors related to the "no_implicit_optional" flag in mypy.ini
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 12, 2021
@michaelbrewer
Copy link
Contributor Author

@heitorlessa with no_implicit_optional=True we have alot more errors coming through in PyCharm.

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2021

Codecov Report

Merging #521 (02fc79b) into develop (c0c32bc) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #521   +/-   ##
========================================
  Coverage    99.23%   99.23%           
========================================
  Files          113      113           
  Lines         4468     4468           
  Branches       243      243           
========================================
  Hits          4434     4434           
  Misses          22       22           
  Partials        12       12           
Impacted Files Coverage Δ
aws_lambda_powertools/logging/formatter.py 100.00% <ø> (ø)
aws_lambda_powertools/logging/logger.py 100.00% <ø> (ø)
...da_powertools/utilities/idempotency/idempotency.py 98.57% <ø> (ø)
...wertools/utilities/idempotency/persistence/base.py 99.36% <ø> (ø)
...ambda_powertools/utilities/validation/validator.py 100.00% <ø> (ø)
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <100.00%> (ø)
aws_lambda_powertools/event_handler/appsync.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metric.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metrics.py 100.00% <100.00%> (ø)
... and 7 more

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 c0c32bc...02fc79b. Read the comment docs.

@michaelbrewer michaelbrewer changed the title fix(mypy): no implicit optional fix(mypy): fixed to resolve no implicit optional errors Jul 13, 2021
@michaelbrewer michaelbrewer changed the title fix(mypy): fixed to resolve no implicit optional errors fix(mypy): fixes to resolve no implicit optional errors Jul 14, 2021
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only one question on whether Optional[Sequence] is the correct type for __build_config as opposed to Union[List, Tuple, None]

aws_lambda_powertools/tracing/tracer.py Outdated Show resolved Hide resolved
@heitorlessa
Copy link
Contributor

I've just tried fixing the Union[Tuple, List] with a Sequence and it worked w/ MyPy -- perhaps I have MyPy misconfigured in PyCharm?

I'll look into other bug now then merge as-is if you're still seeing the issue.

from typing import Any, Callable, Dict, List, Optional, Tuple, Union, Sequence

    def __init__(
        self,
        service: Optional[str] = None,
        disabled: Optional[bool] = None,
        auto_patch: Optional[bool] = None,
        patch_modules: Optional[Sequence[str]] = None,
        provider: Optional[BaseProvider] = None,
    ):

    def patch(self, modules: Optional[Sequence[str]] = None):
      
    def __build_config(
        self,
        service: Optional[str] = None,
        disabled: Optional[bool] = None,
        auto_patch: Optional[bool] = None,
        patch_modules: Optional[Sequence[str]] = None,
        provider: Optional[BaseProvider] = None,
    ):

I've also added the following modules to ignore their lack of typehints, which makes me wonder how reliable this is: https://pypi.org/project/mypy-boto3/

[mypy-boto3]
ignore_missing_imports = True

[mypy-boto3.dynamodb.conditions]
ignore_missing_imports = True

[mypy-botocore.config]
ignore_missing_imports = True

[mypy-botocore.exceptions]
ignore_missing_imports = True

[mypy-aws_xray_sdk.ext.aiohttp.client]
ignore_missing_imports = True

@michaelbrewer
Copy link
Contributor Author

@heitorlessa ok your suggestions work prefectly, i have pushed the latest

@heitorlessa heitorlessa added this to the 1.18.0 milestone Jul 16, 2021
@heitorlessa heitorlessa merged commit 5b87bb1 into aws-powertools:develop Jul 16, 2021
@michaelbrewer michaelbrewer deleted the mypy-implicit-optional branch July 16, 2021 18:49
heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Jul 17, 2021
* develop:
  chore(deps): bump boto3 from 1.18.0 to 1.18.1 (aws-powertools#528)
  fix(tracer): mypy generic to preserve decorated method signature (aws-powertools#529)
  fix(parser): Make ApiGateway version, authorizer fields optional (aws-powertools#532)
  fix(mypy): fixes to resolve no implicit optional errors (aws-powertools#521)
  chore(deps): bump boto3 from 1.17.110 to 1.18.0 (aws-powertools#527)
  feat(feat-toggle): New simple feature toggles rule engine (WIP) (aws-powertools#494)
  chore(deps-dev): bump mkdocs-material from 7.1.9 to 7.1.10 (aws-powertools#522)
  chore(deps): bump boto3 from 1.17.102 to 1.17.110 (aws-powertools#523)
  chore(deps-dev): bump isort from 5.9.1 to 5.9.2 (aws-powertools#514)
  feat(mypy): add mypy support to makefile (aws-powertools#508)
  feat(api-gateway): add debug mode (aws-powertools#507)
heitorlessa added a commit to whardier/aws-lambda-powertools-python that referenced this pull request Jul 19, 2021
…ent-subclass

* develop:
  fix(api-gateway): non-greedy route pattern regex (aws-powertools#533)
  chore(deps): bump boto3 from 1.18.0 to 1.18.1 (aws-powertools#528)
  fix(tracer): mypy generic to preserve decorated method signature (aws-powertools#529)
  fix(parser): Make ApiGateway version, authorizer fields optional (aws-powertools#532)
  fix(mypy): fixes to resolve no implicit optional errors (aws-powertools#521)
  chore(deps): bump boto3 from 1.17.110 to 1.18.0 (aws-powertools#527)
@heitorlessa heitorlessa changed the title fix(mypy): fixes to resolve no implicit optional errors fix(mypy): addresses lack of optional types Jul 20, 2021
@heitorlessa heitorlessa added the bug Something isn't working label Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants