-
-
Notifications
You must be signed in to change notification settings - Fork 810
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 built-in function docs #604
Conversation
docs/built-in-functions.rst
Outdated
|
||
Functions | ||
========= | ||
The first list dictates each argument and the second sublist lists the types allowed as input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe consider some simple examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact linking to the online execution environment (compiler / remix etc...) with executable examples in context would be super useful to developers but that would require writing those examples so should be a separate task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a fan of simple examples, down the road maybe we could have a section in examples/
dedicated to showing off specific viper functionality?
docs/built-in-functions.rst
Outdated
:param a: value to round down | ||
:type a: either decimal or num | ||
|
||
:output b: interger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: interger -> integer
docs/built-in-functions.rst
Outdated
:output product: num256[2] | ||
""" | ||
|
||
Takes two elliptical curves and mupltiples them together. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: mupltiples -> multiplies
docs/built-in-functions.rst
Outdated
""" | ||
:param a: pair to be multipled | ||
:type a: num252[2] | ||
:param b: pair to be multipled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: multipled -> multiplied
docs/built-in-functions.rst
Outdated
|
||
def ecadd(a, b) -> product: | ||
""" | ||
:param a: pair to be multipled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: multipled -> multiplied
docs/built-in-functions.rst
Outdated
|
||
Functions | ||
========= | ||
The first list dictates each argument and the second sublist lists the types allowed as input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type information appears to be available in the signature annotations. Would it be possible to utilize that info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just removed this line as I think it just creates more confusion then it's worth.
docs/built-in-functions.rst
Outdated
|
||
def extract32(a, b, type=c) -> d: | ||
""" | ||
:param a: where 32 bytes are extacted from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling: extacted -> extracted
""" | ||
Returns the length of a given list of bytes. | ||
|
||
* **concat** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concat appears to lack the signature annotation in functions.py (Is this problematic?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a problem, it doesn't have signature annotations because it takes a dynamic number of arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought that might be the case. On inspection of the code it looks ok, as concat checks its own argument semantics.
def as_num128(a) -> b: | ||
""" | ||
:param a: value to turn into int128 | ||
:type a: either num, bytes32, num256, or bytes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider specific examples as to how this transformation occurs, in order to avoid surprises and sneaky bugs for developers.
docs/built-in-functions.rst
Outdated
|
||
:output b: bytes | ||
""" | ||
**Name:** ``method_id`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
markup
* **decimal** | ||
:: | ||
|
||
def decimal(a) -> b: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name is different from other type conversion functions. Is this an intensional design choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it doesn't seem to be included in the types.rst conversion table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to change the name to as_decimal
:param a: value to turn into decimal | ||
:type a: either decimal or num | ||
|
||
:output b: decimal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these type conversions we should be sure to document general behavior and corner cases (e.g. underflow / overflow behavior)
docs/built-in-functions.rst
Outdated
|
||
:output d: bytes | ||
""" | ||
Takes a a list of bytes, the start bytes to take and the length to take and returns the associated chunk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied or referenced? (Possible source of sneaky bugs)
* **concat** | ||
:: | ||
|
||
def concat(a, b, ...) -> c: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation of variadic args here (...). We should document constraints here (e.g. concat requires two or more arguments of type byte32 or byte[])
def bytes_to_num(a) -> b: | ||
""" | ||
:param a: bytes to be transformed | ||
:type a: bytes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider documenting the behavior when arrays are length 0 if that is valid input
docs/built-in-functions.rst
Outdated
* **slice** | ||
:: | ||
|
||
def slice(a, start=b, length=c) -> d: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider documenting the behavior when the range is invalid for the given input
1a2fa0d
to
077aa82
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @fordacious!
docs/built-in-functions.rst
Outdated
|
||
:output d: bytes | ||
""" | ||
Takes a a list of bytes, the start bytes to take and the length to take and returns the associated chunk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo:
Takes a a list of bytes
""" | ||
|
||
Takes a function declaration and returns its method_id (used in data field to call it). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes there are two spaces between annotations, other times there are one. Intentional?
077aa82
to
3c354b0
Compare
3c354b0
to
9a218f7
Compare
@fordacious @sdtsui Thanks a lot for the feedback, I corrected the typos and tried to make a few parts clearer. Merging this in. |
- What I did
Created a docs page for Vipers built-in functions
- How I did it
Looked over the functions in
functions.py
and wrote documentation for them.- How to verify it
Read the documentation
- Description for the changelog
None
- Cute Animal Picture