Simplify arrow utility by renaming to more descriptive names #10123
Conversation
lukasmasuch
added
security-assessment-completed
lukasmasuch
changed the title
| * NOTE: ArrowJS automatically formats the columns in schema, i.e. we always get strings. | ||
| */ | ||
| export type Columns = string[][] | ||
| export type ColumnNames = string[][] |
There was a problem hiding this comment.
Since you are about to update the comments: maybe this comment could be updated with a quick info why this is a multidimensional array. I suppose it is for nested column names?
There was a problem hiding this comment.
Yeah, to support multi-level column headers. Added it to the comment 👍
|
|
||
| /** DataFrame index and data types. */ | ||
| export interface Types { | ||
| /** DataFrame's index and data column types. */ |
There was a problem hiding this comment.
super-nit: /** A DataFrame's ...
| /** DataFrame index and data types. */ | ||
| export interface Types { | ||
| /** DataFrame's index and data column types. */ | ||
| export interface ColumnTypes { |
There was a problem hiding this comment.
Since this interface seems to only support Pandas types, should it maybe be called PandasColumnTypes similar to the other renamings? And will this be generalized more in a follow-up step?
There was a problem hiding this comment.
Renamed 👍
And will this be generalized more in a follow-up step?
Yep, there is still quite a bit of reliance on pandas metadata to remove to support purely raw arrow data. That will come with follow-up PRs. Goal is that the Pandas metadata is only used for additional accuracy, but the main information should come from the Arrow metadata. Unfortunately, the reliance on pandas' metadata was even more extreme than I had initially thought.
raethlein
left a comment
raethlein
left a comment
There was a problem hiding this comment.
LGTM, thanks for making the names clearer!
…it#10123) ## Describe your changes This PR simplifies the arrow handling logic by renaming variables and functions to more descriptive names. This also adds a variety of code comments to make it easier to understand the logic. To simplify reviewing, this PR does not apply any logical changes. ## Testing Plan - No logical or visual changes - no new tests required. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
Describe your changes
This PR simplifies the arrow handling logic by renaming variables and functions to more descriptive names. This also adds a variety of code comments to make it easier to understand the logic.
To simplify reviewing, this PR does not apply any logical changes.
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.