Navigation Menu

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

feat: arithmetic for new int types #2843

Merged
merged 48 commits into from Jun 17, 2022

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented May 5, 2022

What I did

implement #2786, fixes #2840

How I did it

refactor arithmetic to be generic over numeric types

How to verify it

check arithmetic tests, they now test over all types and "interesting" values. also added more interesting decimal tests.

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

image

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 5, 2022

This pull request introduces 3 alerts when merging ea16bf0 into df2da62 - view on LGTM.com

new alerts:

  • 2 for Unreachable code
  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 5, 2022

This pull request introduces 2 alerts when merging 8fef032 into df2da62 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 6, 2022

This pull request introduces 2 alerts when merging 2324bcf into 3cbdf35 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #2843 (719aac7) into master (a37bbbc) will increase coverage by 0.13%.
The diff coverage is 80.18%.

@@            Coverage Diff             @@
##           master    #2843      +/-   ##
==========================================
+ Coverage   88.17%   88.30%   +0.13%     
==========================================
  Files          95       96       +1     
  Lines       10584    10611      +27     
  Branches     2534     2531       -3     
==========================================
+ Hits         9332     9370      +38     
+ Misses        792      787       -5     
+ Partials      460      454       -6     
Impacted Files Coverage Δ
vyper/codegen/types/types.py 88.34% <75.00%> (ø)
vyper/codegen/arithmetic.py 75.30% <75.30%> (ø)
vyper/codegen/expr.py 83.72% <88.23%> (+4.29%) ⬆️
vyper/builtin_functions/convert.py 90.60% <100.00%> (ø)
vyper/codegen/core.py 84.58% <100.00%> (+0.38%) ⬆️
vyper/ir/optimizer.py 92.22% <100.00%> (+0.37%) ⬆️
vyper/utils.py 89.07% <100.00%> (+1.15%) ⬆️

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 a37bbbc...719aac7. Read the comment docs.

@charles-cooper charles-cooper force-pushed the refactor_arithmetic branch 3 times, most recently from 287b82d to 2d49d2f Compare May 6, 2022 09:21
@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 6, 2022

This pull request introduces 2 alerts when merging 2d49d2f into 3cbdf35 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 6, 2022

This pull request introduces 6 alerts when merging 379fac9 into 3cbdf35 - view on LGTM.com

new alerts:

  • 3 for Unused local variable
  • 2 for Unreachable code
  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 6, 2022

This pull request introduces 3 alerts when merging ac8454d into 3cbdf35 - view on LGTM.com

new alerts:

  • 2 for Unreachable code
  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 7, 2022

This pull request introduces 3 alerts when merging a338657 into 31e2b4e - view on LGTM.com

new alerts:

  • 2 for Unreachable code
  • 1 for Unused import

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 9, 2022

This pull request introduces 2 alerts when merging 4d32b08 into 18b9083 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 9, 2022

This pull request introduces 2 alerts when merging ef11232 into 18b9083 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 9, 2022

This pull request introduces 3 alerts when merging c29128f into 18b9083 - view on LGTM.com

new alerts:

  • 2 for Unreachable code
  • 1 for Redundant assignment

@charles-cooper charles-cooper changed the title Refactor arithmetic feat: arithmetic for new int types May 15, 2022
xs = special_cases.copy()
ys = special_cases.copy()
NUM_CASES = 10
# poor man's fuzzing - hypothesis doesn't make it easy
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

@charles-cooper
Copy link
Member Author

it's a bit odd, tox reports that no tests were collected for group 45, but when i run that group directly via pytest, it runs 316 tests.

the (default) duration_based_chunks splitting algorithm in pytest-split
resulted in a 0-size chunk getting allocated to the final group.
@charles-cooper charles-cooper enabled auto-merge (squash) June 17, 2022 17:00
@charles-cooper charles-cooper merged commit daeae11 into vyperlang:master Jun 17, 2022
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.

Obscure message for UnimplementedException
4 participants