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: remove unnecessary typing-extensions for py3.8 #281

Merged

Conversation

nadobando
Copy link
Contributor

@nadobando nadobando commented Feb 3, 2021

Issue #280

Description of changes:

  1. Added if block to know if typing_extensions should be imported based on the python version.
    if python >= 3.8 typing extensions will not be imported and the native implementation will be used
    else will try to import typing_extensions and catch the ImportError

  2. Updated modules which imported this dependency.

  1. Updated pyproject.toml to only install typing-extensions when python version is <3.8
    Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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

@nadobando nadobando force-pushed the fix/remove-typing-ext-for-py38 branch from c1e7324 to fc78bea Compare February 3, 2021 14:44
@codecov-io
Copy link

codecov-io commented Feb 3, 2021

Codecov Report

Merging #281 (fc78bea) into develop (575a103) will decrease coverage by 0.06%.
The diff coverage is 84.61%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #281      +/-   ##
===========================================
- Coverage    99.75%   99.68%   -0.07%     
===========================================
  Files           79       79              
  Lines         2839     2861      +22     
  Branches       118      120       +2     
===========================================
+ Hits          2832     2852      +20     
- Misses           5        7       +2     
  Partials         2        2              
Impacted Files Coverage Δ
aws_lambda_powertools/utilities/parser/types.py 80.00% <71.42%> (-20.00%) ⬇️
...bda_powertools/utilities/parser/models/dynamodb.py 100.00% <100.00%> (ø)
...mbda_powertools/utilities/parser/models/kinesis.py 100.00% <100.00%> (ø)
...ws_lambda_powertools/utilities/parser/models/s3.py 100.00% <100.00%> (ø)
...s_lambda_powertools/utilities/parser/models/ses.py 100.00% <100.00%> (ø)
...s_lambda_powertools/utilities/parser/models/sns.py 100.00% <100.00%> (ø)
...s_lambda_powertools/utilities/parser/models/sqs.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/batch/sqs.py 100.00% <0.00%> (ø)
aws_lambda_powertools/utilities/batch/base.py 100.00% <0.00%> (ø)
...ws_lambda_powertools/utilities/batch/exceptions.py 100.00% <0.00%> (ø)

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 575a103...fc78bea. Read the comment docs.

@nadobando nadobando force-pushed the fix/remove-typing-ext-for-py38 branch from fc78bea to f584ed4 Compare February 3, 2021 14:51
Copy link
Contributor

@to-mc to-mc left a comment

Choose a reason for hiding this comment

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

Nice change, great to keep the package size down where we can! Added a small suggestion to change - otherwise LGTM!

aws_lambda_powertools/utilities/parser/types.py Outdated Show resolved Hide resolved
aws_lambda_powertools/utilities/parser/types.py Outdated Show resolved Hide resolved
@nadobando nadobando force-pushed the fix/remove-typing-ext-for-py38 branch from f584ed4 to 11627ff Compare February 3, 2021 16:38
@nadobando nadobando requested a review from to-mc February 3, 2021 16:42
@heitorlessa
Copy link
Contributor

Thanks a lot for this contribution @nadobando !!! And thanks @cakepietoast for the review. Merging this and publishing a new release

@heitorlessa heitorlessa merged commit 2591050 into aws-powertools:develop Feb 4, 2021
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.

None yet

4 participants