wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-640250180
Closing the PR again. Please open JIRA issues or review other PRs to provide
more feedback.
This is an automated
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-636402690
I'll leave this PR open until next week sometime to allow more time for
comments
This is an automated message from
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633240348
The PR has been merged, but the code review feature is still available.
Please continue to leave comments and I will address them
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633231974
+1
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633229487
@jorisvandenbossche I fixed the segfault you hit and now have changed the
implementation of `Array.filter` to be
```
pc = _pc()
options =
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633218315
@jorisvandenbossche I'll take a look at that segfault
> The goal is to eventually have them all go through _pc.call_function()?
Yes. `arrow::compute::CallFunction` is
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633154141
I'd like to work on a couple follow-up PRs tomorrow, so in order to unblock
these PRs so that they can be reviewed, this PR will have to be merged first.
So I would like to do that
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-633051554
The build is passing (the Travis thing is a flake on ARM architecture),
thanks @kou for your help with that! I think we should merge this today and I
will continue to address
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632975883
I see the problem, I am fixing
This is an automated message from the Apache Git Service.
To respond to the message,
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632975274
Ok I will take a look
This is an automated message from the Apache Git Service.
To respond to the message, please
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632947370
Note that `kHashSeed` is declared as `uint64_t` so that must be changed also
```
static constexpr uint64_t kHashSeed = 0;
```
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632947188
@kou that's fine, please go ahead and apply it
This is an automated message from the Apache Git Service.
To respond
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632945792
Perfect thank you :pray:. I'm done hacking on this for now, I'll keep
investigating test failures
This is an
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632945052
I'm just fixing a failing unit test now.
This is an automated message from the Apache Git Service.
To respond to
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632944653
@xhochy @pitrou @kszucs @jorisvandenbossche I just added Python bindings
for this new functionality including generic argument packing and function
dispatching. The new
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632937311
Done. It looks like GitHub is having some problems today but will keep an
eye on the builds
This is an automated
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632936766
OK, I have it fixed locally. I'm finishing some Python stuff and then I'll
push
This is an automated message from
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632936071
Thank you, I will fix.
This is an automated message from the Apache Git Service.
To respond to the message, please
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632929465
@kou done, thank you for catching this, I think it's better to retain this
functionality.
This is an automated
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632927540
The IsIn/Match changes don't look too hard, should be able to push them in
the next half hour or less
This is an
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632925674
I'm taking a look now, I'll report back in a half hour or so
This is an automated message from the Apache Git
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632925336
@kou yes, that was an API change. I can try to restore this functionality
though?
This is an automated message from
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632919447
Awesome, thanks @kou!
This is an automated message from the Apache Git Service.
To respond to the message, please
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632918704
The R test suite is fixed -- this also turned up some missing test coverage
for things that R was depending on
https://issues.apache.org/jira/browse/ARROW-8895.
I'm going to
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632910238
This problem of kernel dispatch with timestamps that may or may not have
time zones actually brought out a limitation with input type checking. I'm
introducing a simple interface
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632824231
I just rebased and pushed MSVC build fixes (that works for me at least
locally on VS 2017). I'm going to fix the R failure next and then address the
other accumulated comments
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632460179
I got sucked into some other things today, I'll focus on getting the CI
passing tomorrow so we can hopefully go into the weekend with a green build
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632452421
@emkornfield @brills no Python APIs are changed (not sure how much of the
C++ API TFX uses)
This is an automated
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632430558
I rebased without pushing any new changes
This is an automated message from the Apache Git Service.
To respond to
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-632167817
@nealrichardson yeah, I will fix
This is an automated message from the Apache Git Service.
To respond to the
wesm commented on pull request #7240:
URL: https://github.com/apache/arrow/pull/7240#issuecomment-631878670
For code reviewers:
* The code diff is a bit deceiving because many kernel files were renamed to
make their taxonomy more clear
* I suggest focusing your energies on the
31 matches
Mail list logo
