-
Notifications
You must be signed in to change notification settings - Fork 251
[hail] hl.range supports a single argument #6903
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
Conversation
This matches the semantics of Python `range`. See docs for example.
hail/python/hail/expr/functions.py
Outdated
>>> hl.eval(hl.range(0, 10, step=3)) | ||
[0, 3, 6, 9] | ||
|
||
Notes | ||
----- | ||
The range includes `start`, but excludes `stop`. | ||
|
||
If provided exactly one argument, the argument is interpreted as `stop` and | ||
`start` is set to zero. This matches the behavior Python's :func:`range`. |
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.
this will link back to this function
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.
If we want a link, I think it will need to be external. Python ranges live here: https://docs.python.org/3/library/stdtypes.html#ranges
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 the link.
self.assertRaisesRegex( | ||
hl.utils.HailException, | ||
'Array range cannot have step size 0', | ||
hl.eval(hl.range(0,1,0) |
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.
Unclosed parens.
hail/python/hail/expr/functions.py
Outdated
>>> hl.eval(hl.range(0, 10, step=3)) | ||
[0, 3, 6, 9] | ||
|
||
Notes | ||
----- | ||
The range includes `start`, but excludes `stop`. | ||
|
||
If provided exactly one argument, the argument is interpreted as `stop` and | ||
`start` is set to zero. This matches the behavior Python's :func:`range`. |
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.
If we want a link, I think it will need to be external. Python ranges live here: https://docs.python.org/3/library/stdtypes.html#ranges
hail/python/hail/expr/functions.py
Outdated
@@ -1910,6 +1916,9 @@ def range(start, stop, step=1) -> ArrayNumericExpression: | |||
------- | |||
:class:`.ArrayInt32Expression` | |||
""" | |||
if stop is None: | |||
stop = start | |||
start = 0 |
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, this isn't an expression and causes type checking errors later.
>>> hl.range(10).show()
File "<string>", line 1, in <module>
File "</home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/decorator.py:decorator-gen-638>", line 2, in range
File "/home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/hail/typecheck/check.py", line 585, in wrapper
return __original_func(*args_, **kwargs_)
File "/home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/hail/expr/functions.py", line 1922, in range
return apply_expr(lambda sta, sto, ste: ArrayRange(sta, sto, ste), tarray(tint32), start, stop, step)
File "/home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/hail/expr/expressions/typed_expressions.py", line 3646, in apply_expr
indices, aggregations = unify_all(*args)
File "/home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/hail/expr/expressions/base_expression.py", line 225, in unify_all
new_indices = Indices.unify(*[e._indices for e in exprs])
File "/home/cdv/.cache/venv/hail/lib64/python3.6/site-packages/hail/expr/expressions/base_expression.py", line 225, in <listcomp>
new_indices = Indices.unify(*[e._indices for e in exprs])
AttributeError: 'int' object has no attribute '_indices'
This matches the semantics of Python
range
. See docs for example.