GH-47390: [C++][Acero] Allow for any type of scalar in Pivot longer features - #47391
Conversation
|
|
|
Thank you for your contribution. Unfortunately, this pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you do not have repository permissions to reopen the PR, please tag a maintainer. |
5198f72 to
324cde6
Compare
|
@zanmato1984 @westonpace @pitrou What do I need to do to get my PRs reviewed? I am waiting over a year with no response. Bot is now trying to remove what I believe valid contribution. |
-fix error type -restore test to use legacy constructor
|
@zanmato1984 Do you think this is a reasonable addition? |
|
@pitrou Yes, I think the capability itself is reasonable. Feature columns produced by pivot-longer can naturally contain typed values, for example integer week numbers, dates, or timestamps. Forcing them through strings loses type information and requires downstream casts. Keeping the I left one inline comment about handling null scalar pointers safely. Also, since the public |
Co-authored-by: Rossi Sun <zanmato1984@gmail.com>
|
@zanmato1984 good catch. I believe I have no permissions to modify labels. Can you add it? |
Rationale for this change
Allow supplying any scalar as feature
What changes are included in this PR?
Support scalar in PivotLonger
Are these changes tested?
Yes
Are there any user-facing changes?
Yes.
This PR includes breaking changes to public APIs.
PivotLongerRowTemplate::feature_valuesis now astd::vector<std::shared_ptr<Scalar>>while it used to bestd::vector<std::string>).The
PivotLongerRowTemplatestill allows passing a vector of strings, though.