[query] Add hl.query_table function for reading tables with indices…
#12376
Conversation
|
Important infrastructure for Seqr. |
tpoterba
force-pushed
the
filtered-read
branch
from
2832ee7 to
f2629ce
Compare
hail/python/hail/expr/functions.py
Outdated
| f"query_table: key mismatch: cannot query a table with key " | ||
| f"({', '.join(builtins.str(x) for x in key_typ.values())}) with query point type {point.dtype}") | ||
|
|
||
| if isinstance(point_or_interval.dtype, hl.tinterval): |
There was a problem hiding this comment.
This shouldn't include the case where the table is keyed by intervals. I think the right logic is something like
if point_or_interval.dtype is a prefix of key_type:
...
elif point_or_interval.dtype == tinterval(key_type[0]):
...
else:
error
hail/python/hail/ir/ir.py
Outdated
| self.row_typ_with_uid = tstruct(**self.row_typ, __uid=ttuple(tint64, tint64)) | ||
| self.drop_uid = False | ||
| self.reader.set_uid_field('__uid') |
There was a problem hiding this comment.
All the other _handle_randomness methods return new nodes, they don't modify the node. I think that's necessary, since an IR can be reused in different contexts.
|
|
||
|
|
||
| def test_query_table(): | ||
| f = new_temp_file(extension='ht') |
There was a problem hiding this comment.
Maybe include a few tests with a compound key?
| ((cb, elt) => elt), | ||
| bound = "lower", | ||
| compareEnd(_, _, _, _ < 1) | ||
| ).search(cb, partitionerRuntime, EmitCode.present(cb.emb, endAndSignTuple))) |
There was a problem hiding this comment.
From a quick look, I think you've reimplemented the registered function partitionerFindIntervalRange. If so, you should just factor out the implementation to a staged function (as most of the interval registered functions are) and call that. I think we should try to keep all the staged interval ordering stuff in one place, since it's relatively complicated.
|
good comments; addressed these and did a fair bit of redesigning of the Python stuff. Refactored the scala to use that method you mentioned, also got mad that we didn't already have a SStackInterval and added that. |
3 participants
… from Python