Skip to content

Clean up tables interface implementation#503

Open
quinnj wants to merge 1 commit intomasterfrom
quinnj-patch-1
Open

Clean up tables interface implementation#503
quinnj wants to merge 1 commit intomasterfrom
quinnj-patch-1

Conversation

@quinnj
Copy link
Member

@quinnj quinnj commented Dec 7, 2021

Reasoning for changes:

  • We don't need to define rowaccess/rows because if we define columnaccess/columns then we automatically get an efficient fallback rows definition
  • We don't need to define columnnames/getcolumn/schema on TimeArray, only on the type that Tables.columns returns, so in this case, just TableIter
  • In the constructor, we need to call Tables.columns first and then call Tables.schema(cols); calling Tables.schema directly on the table input isn't valid, as it wouldn't work in, for example, the case where someone passed in a ::TimeArray.

@iblislin
Copy link
Collaborator

The reason that we defined schema is for the time column.
Since the fallback function return only the data column. for example, [:a, :b, :c], but we defined the custom schema to return [:timestamp, :a, :b, :c]

@quinnj
Copy link
Member Author

quinnj commented Jan 11, 2022

The reason that we defined schema is for the time column. Since the fallback function return only the data column. for example, [:a, :b, :c], but we defined the custom schema to return [:timestamp, :a, :b, :c]

Yep, that's perfectly fine/valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants