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

docs: new for loop range syntax: bound= #3540

Merged
merged 9 commits into from
Sep 29, 2023

Conversation

charles-cooper
Copy link
Member

for i in range(..., bound=...)

What I did

How I did it

How to verify it

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

Put a link to a cute animal picture inside the parenthesis-->

for i in range(..., bound=...)
@charles-cooper charles-cooper requested a review from fubuloubu July 26, 2023 16:56
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Merging #3540 (4fc3778) into master (1711569) will decrease coverage by 0.16%.
Report is 10 commits behind head on master.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #3540      +/-   ##
==========================================
- Coverage   89.12%   88.97%   -0.16%     
==========================================
  Files          86       86              
  Lines       11407    11460      +53     
  Branches     2595     2606      +11     
==========================================
+ Hits        10166    10196      +30     
- Misses        819      839      +20     
- Partials      422      425       +3     
Files Coverage Δ
vyper/codegen/stmt.py 89.92% <ø> (ø)
vyper/ir/compile_ir.py 92.57% <ø> (ø)

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more


for i in range(min(stop, N), bound=N):
...
``stop`` can be any integer greater than zero. ``N`` must be a compile-time constant. ``i`` begins as zero and increments by one until it is equal to ``stop``. If ``stop`` is larger than ``N``, execution will revert at runtime. In certain cases, you may not have a guarantee that ``stop`` is less than ``N``, but still want to avoid the possibility of runtime reversion. To accomplish this, use the ``bound=`` keyword in combination with ``min()``, like ``range(min(stop, N), bound=N)``.
Copy link
Member

Choose a reason for hiding this comment

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

If it's zero will it raise? Or avoid the loop?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``stop`` can be any integer greater than zero. ``N`` must be a compile-time constant. ``i`` begins as zero and increments by one until it is equal to ``stop``. If ``stop`` is larger than ``N``, execution will revert at runtime. In certain cases, you may not have a guarantee that ``stop`` is less than ``N``, but still want to avoid the possibility of runtime reversion. To accomplish this, use the ``bound=`` keyword in combination with ``min()``, like ``range(min(stop, N), bound=N)``.
``stop`` can be any integer greater than zero. ``N`` must be a compile-time constant. ``i`` begins as zero and increments by one until it is equal to ``stop``. If ``stop`` is larger than ``N``, execution will revert at runtime. In certain cases, you may not have a guarantee that ``stop`` is less than ``N``, but still want to avoid the possibility of runtime reversion. To accomplish this, use the ``bound=`` keyword in combination with ``min(stop, N)`` as the argument to ``range``, like ``range(min(stop, N), bound=N)``. This is helpful for use cases like chunking up operations on larger arrays across multiple transactions.

Copy link
Member

Choose a reason for hiding this comment

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

If it's zero will it raise? Or avoid the loop?

Sorry @charles-cooper, made two comments at once here lol

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's zero will it raise? Or avoid the loop?

it will skip the loop. again, this is clear in the tests from the implementing PR d48438e#diff-c3f9a43de8dca93393504fef8b3c6bd8c77a6be969ee749f59b49537285e6392R27 but i've updated it to be just a little more clear in b613cb9

Copy link
Member

Choose a reason for hiding this comment

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

Can add that note to the docs too?

Copy link
Collaborator

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

This is how the current docs look like:

image

IMO the start of the entire block with for i in range(stop, bound=N): is confusing since there is no introduction but just simply starts with a code block.

We should have something like:

Furthermore, the range function allows for the kwarg bound in order to use ranges with max bounds:

    for i in range(stop, bound=N):
        ...

[The rest of the text]

vyper/codegen/stmt.py Show resolved Hide resolved
@charles-cooper charles-cooper enabled auto-merge (squash) September 29, 2023 00:45
@charles-cooper charles-cooper changed the title docs: new for loop range syntax docs: new for loop range syntax: bound= Sep 29, 2023
@charles-cooper charles-cooper enabled auto-merge (squash) September 29, 2023 00:46
@charles-cooper charles-cooper merged commit 917959e into vyperlang:master Sep 29, 2023
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.

5 participants