-
Notifications
You must be signed in to change notification settings - Fork 243
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
[hail] Add table and matrix table indices #6266
Changes from all commits
35876d0
1f8ddf6
1786b78
f1f1ee1
2cf177c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,7 @@ def maximal_independent_set(i, j, keep=True, tie_breaker=None, keyed=True) -> Ta | |
|
||
When multiple nodes have the same degree, this algorithm will order the | ||
nodes according to ``tie_breaker`` and remove the *largest* node. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whitespace change, and not a fix to something against default formatting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My editor does this automatically. I can revert this in particular if we want, but also I feel that it's good style to not have trailing whitespace. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I'm sympathetic, but it does make the diff harder to read. A setting that applies the transformation only in lines otherwise modified would be great -- does that exist? |
||
|
||
If `keyed` is ``False``, then a node may appear twice in the resulting | ||
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.
I think this logic should be in read_matrix_table -- we generally assume that parameters on the IR have been validated.
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, shouldn't there be a check that the interval type agrees with the key type?
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.
Can't do that until this is wrapped in a
Table
/MatrixTable
(python classes). We have no access to the key type at this point. I can certainly move much of the logic intoread(_matrix)_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.
Yeah, realized that after I commented and was looking at some of the code i master. I think it would be preferable to do the type lookup in read_matrix_table and then pass it into the IR node, but that's definitely out of scope of this change.