Issues around auto visualization row/column preference specification #17
Conversation
vyeevani
commented
Aug 7, 2024
vyeevani
commented
- Fixing bugs in the named array row/column reference
- Adding support for ordering inheritance from the preferred row and column list.
- Adding support for ordering inheritance from the preferred row and column list.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| for column_key in self.prefers_column: | ||
| for info in array_axis_info: | ||
| if isinstance(info, NamedPositionalAxisInfo | NamedPositionlessAxisInfo | PositionalAxisInfo): | ||
| if info.logical_key() == column_key: | ||
| column_axes.append(info) | ||
|
|
||
| for row_key in self.prefers_row: | ||
| for info in array_axis_info: | ||
| if isinstance(info, NamedPositionalAxisInfo | NamedPositionlessAxisInfo | PositionalAxisInfo): | ||
| if info.logical_key() == row_key: | ||
| row_axes.append(info) |
There was a problem hiding this comment.
Why change this to a double loop? I think the original loop structure will be faster and avoid unnecessary checks for axes that are not named.
Also, are you trying to add support for preferring integer axis positions? I don't think this works correctly since NamedPositionalAxisInfo has both a name and a position and the logical_key just returns one of them.
| if isinstance(ax, PositionalAxisInfo | NamedPositionalAxisInfo): | ||
| return (0, -ax.axis_logical_index) | ||
| elif ax in unassigned: | ||
| if ax in unassigned: |
